Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(hashing): improve byte array conversion methods #2279

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
10 changes: 6 additions & 4 deletions mm2src/coins/lightning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use secp256k1v24::PublicKey;
use serde::Deserialize;
use serde_json::Value as Json;
use std::collections::{HashMap, HashSet};
use std::convert::TryInto;
use std::convert::{TryFrom, TryInto};
use std::fmt;
use std::io::Cursor;
use std::net::SocketAddr;
Expand Down Expand Up @@ -1046,7 +1046,8 @@ impl MarketCoinOps for LightningCoin {
.map_err(|_| SignatureError::InternalError("Error accessing node keys".to_string()))?;
let private = Private {
prefix: 239,
secret: H256::from(*secret_key.as_ref()),
secret: H256::from_slice(secret_key.as_ref())
.map_to_mm(|err| SignatureError::InvalidRequest(err.to_string()))?,
compressed: true,
checksum_type: ChecksumType::DSHA256,
};
Expand All @@ -1058,10 +1059,11 @@ impl MarketCoinOps for LightningCoin {
let message_hash = self
.sign_message_hash(message)
.ok_or(VerificationError::PrefixNotFound)?;
let signature = CompactSignature::from(
let signature = CompactSignature::try_from(
zbase32::decode_full_bytes_str(signature)
.map_err(|e| VerificationError::SignatureDecodingError(e.to_string()))?,
);
)
.map_to_mm(|err| VerificationError::SignatureDecodingError(err.to_string()))?;
let recovered_pubkey = Public::recover_compact(&H256::from(message_hash), &signature)?;
Ok(recovered_pubkey.to_string() == pubkey)
}
Expand Down
4 changes: 2 additions & 2 deletions mm2src/coins/qrc20/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl Qrc20Coin {
let secret_hash = if secret_hash.len() == 32 {
ripemd160(secret_hash)
} else {
chain::hash::H160::from(secret_hash)
chain::hash::H160::from_slice(secret_hash)?
};

let spend_tx: UtxoTx = try_s!(deserialize(spend_tx).map_err(|e| ERRL!("{:?}", e)));
Expand Down Expand Up @@ -970,7 +970,7 @@ fn find_receiver_spend_with_swap_id_and_secret_hash(
let expected_secret_hash = if expected_secret_hash.len() == 32 {
ripemd160(expected_secret_hash)
} else {
chain::hash::H160::from(expected_secret_hash)
chain::hash::H160::from_slice(expected_secret_hash).expect("this shouldn't fail")
};

for (output_idx, output) in tx.outputs.iter().enumerate() {
Expand Down
9 changes: 5 additions & 4 deletions mm2src/coins/utxo/slp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use script::{Builder as ScriptBuilder, Opcode, Script, TransactionInputSigner};
use serde_json::Value as Json;
use serialization::{deserialize, serialize, Deserializable, Error as SerError, Reader};
use serialization_derive::Deserializable;
use std::convert::TryInto;
use std::convert::{TryFrom, TryInto};
use std::num::TryFromIntError;
use std::sync::atomic::{AtomicU64, Ordering as AtomicOrdering};
use std::sync::Arc;
Expand Down Expand Up @@ -922,7 +922,7 @@ impl Deserializable for SlpTransaction {
let additional_token_quantity = u64::from_be_bytes(bytes.try_into().expect("length is 8 bytes"));

Ok(SlpTransaction::Mint {
token_id: H256::from(maybe_id.as_slice()),
token_id: H256::from_slice(maybe_id.as_slice()).map_err(|_| SerError::MalformedData)?,
mint_baton_vout,
additional_token_quantity,
})
Expand All @@ -936,7 +936,7 @@ impl Deserializable for SlpTransaction {
)));
}

let token_id = H256::from(maybe_id.as_slice());
let token_id = H256::from_slice(maybe_id.as_slice()).map_err(|_| SerError::MalformedData)?;
let mut amounts = Vec::with_capacity(1);
while !reader.is_finished() {
let bytes: Vec<u8> = reader.read_list()?;
Expand Down Expand Up @@ -1130,7 +1130,8 @@ impl MarketCoinOps for SlpToken {
let message_hash = self
.sign_message_hash(message)
.ok_or(VerificationError::PrefixNotFound)?;
let signature = CompactSignature::from(STANDARD.decode(signature)?);
let signature = CompactSignature::try_from(STANDARD.decode(signature)?)
.map_to_mm(|err| VerificationError::SignatureDecodingError(err.to_string()))?;
let pubkey = Public::recover_compact(&H256::from(message_hash), &signature)?;
let address_from_pubkey = self.platform_coin.address_from_pubkey(&pubkey);
let slp_address = self
Expand Down
4 changes: 3 additions & 1 deletion mm2src/coins/utxo/utxo_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use serialization::{deserialize, serialize, serialize_with_flags, CoinVariant, C
SERIALIZE_TRANSACTION_WITNESS};
use std::cmp::Ordering;
use std::collections::hash_map::{Entry, HashMap};
use std::convert::TryFrom;
use std::str::FromStr;
use std::sync::atomic::Ordering as AtomicOrdering;
use utxo_signer::with_key_pair::{calc_and_sign_sighash, p2sh_spend, signature_hash_to_sign, SIGHASH_ALL,
Expand Down Expand Up @@ -2644,7 +2645,8 @@ pub fn verify_message<T: UtxoCommonOps>(
address: &str,
) -> VerificationResult<bool> {
let message_hash = sign_message_hash(coin.as_ref(), message).ok_or(VerificationError::PrefixNotFound)?;
let signature = CompactSignature::from(STANDARD.decode(signature_base64)?);
let signature = CompactSignature::try_from(STANDARD.decode(signature_base64)?)
.map_to_mm(|err| VerificationError::SignatureDecodingError(err.to_string()))?;
let recovered_pubkey = Public::recover_compact(&H256::from(message_hash), &signature)?;
let received_address = checked_address_from_str(coin, address)?;
Ok(AddressHashEnum::from(recovered_pubkey.address_hash()) == *received_address.hash())
Expand Down
17 changes: 11 additions & 6 deletions mm2src/mm2_bitcoin/crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,39 +34,44 @@ pub enum ChecksumType {
pub fn ripemd160(input: &[u8]) -> H160 {
let mut hasher = Ripemd160::new();
hasher.update(input);
(*hasher.finalize()).into()
let array: [u8; 20] = hasher.finalize().into();
array.into()
}

/// SHA-1
#[inline]
pub fn sha1(input: &[u8]) -> H160 {
let mut hasher = Sha1::default();
let mut hasher = Sha1::new();
hasher.update(input);
(*hasher.finalize()).into()
let array: [u8; 20] = hasher.finalize().into();
array.into()
}

/// SHA-256
#[inline]
pub fn sha256(input: &[u8]) -> H256 {
let mut hasher = Sha256::new();
hasher.update(input);
(*hasher.finalize()).into()
let array: [u8; 32] = hasher.finalize().into();
array.into()
}

/// Groestl-512
#[inline]
pub fn groestl512(input: &[u8]) -> H512 {
let mut hasher = Groestl512::new();
hasher.update(input);
(*hasher.finalize()).into()
let array: [u8; 64] = hasher.finalize().into();
array.into()
}

/// Keccak-256
#[inline]
pub fn keccak256(input: &[u8]) -> H256 {
let mut hasher = Keccak256::new();
hasher.update(input);
(*hasher.finalize()).into()
let array: [u8; 32] = hasher.finalize().into();
array.into()
}

/// Double Keccak-256
Expand Down
11 changes: 8 additions & 3 deletions mm2src/mm2_bitcoin/keys/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

use hash::H520;
use hex::{FromHex, ToHex};
use std::{fmt, ops, str};
use std::convert::TryInto;
use std::{array::TryFromSliceError, convert::TryFrom, fmt, ops, str};
use Error;

#[derive(PartialEq, Clone)]
Expand Down Expand Up @@ -91,6 +92,10 @@ impl From<H520> for CompactSignature {
fn from(h: H520) -> Self { CompactSignature(h) }
}

impl From<Vec<u8>> for CompactSignature {
fn from(v: Vec<u8>) -> Self { CompactSignature(H520::from(&v[..])) }
impl TryFrom<Vec<u8>> for CompactSignature {
type Error = TryFromSliceError;
fn try_from(value: Vec<u8>) -> Result<Self, Self::Error> {
let bytes: &[u8; 65] = &value.as_slice().try_into()?;
Ok(CompactSignature(H520::from(bytes)))
}
}
32 changes: 22 additions & 10 deletions mm2src/mm2_bitcoin/primitives/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use bitcoin_hashes::{sha256d, Hash as ExtHash};
use hex::{FromHex, FromHexError, ToHex};
use std::convert::TryInto;
use std::hash::{Hash, Hasher};
use std::{cmp, fmt, ops, str};

Expand Down Expand Up @@ -39,10 +40,21 @@ macro_rules! impl_hash {
fn from(h: $name) -> Self { h.0 }
}

impl<'a> From<&'a [u8; $size]> for $name {
fn from(slc: &[u8; $size]) -> Self {
let mut inner = [0u8; $size];
inner.copy_from_slice(slc);
$name(inner)
}
}

// DANGEROUS: This will panic if slice length != $size
// Use from_slice() instead which safely checks the length and returns Result
impl<'a> From<&'a [u8]> for $name {
fn from(slc: &[u8]) -> Self {
assert_eq!(slc.len(), $size, "Input slice must be exactly {} bytes", $size);
let mut inner = [0u8; $size];
inner[..].clone_from_slice(&slc[0..$size]);
inner.copy_from_slice(slc);
$name(inner)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still used in a lot of places and we don't know if all the passed slices are checked for size or not, I already started removing this and depending on From<&'a [u8; $size]> instead. Will push a commit once done and will allow someone else to review this PR.

Expand All @@ -61,17 +73,9 @@ macro_rules! impl_hash {

impl str::FromStr for $name {
type Err = FromHexError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let vec: Vec<u8> = s.from_hex()?;
match vec.len() {
$size => {
let mut result = [0u8; $size];
result.copy_from_slice(&vec);
Ok($name(result))
},
_ => Err(FromHexError::InvalidHexLength),
}
Self::from_slice(&vec).map_err(|_| FromHexError::InvalidHexLength)
}
}

Expand Down Expand Up @@ -143,6 +147,14 @@ macro_rules! impl_hash {
pub fn size() -> usize { $size }

pub fn is_zero(&self) -> bool { self.0.iter().all(|b| *b == 0) }

/// Preferred method for constructing from a slice - checks length and returns Result
pub fn from_slice(slc: &[u8]) -> Result<Self, &'static str> {
let bytes: [u8; $size] = slc
.try_into()
.map_err(|_| "Slice length must be exactly 40 bytes")?;
Ok(bytes.into())
}
}
};
}
Expand Down
29 changes: 17 additions & 12 deletions mm2src/mm2_bitcoin/script/src/script.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Serialized script, used inside transaction inputs and outputs.

use bytes::Bytes;
use chain::hash::{H160, H256};
use keys::{self, AddressHashEnum, Public};
use std::{fmt, ops};
use {Error, Opcode};
Expand Down Expand Up @@ -425,12 +426,14 @@ impl Script {
))]
})
},
ScriptType::PubKeyHash => Ok(vec![ScriptAddress::new_p2pkh(AddressHashEnum::AddressHash(
self.data[3..23].into(),
))]),
ScriptType::ScriptHash => Ok(vec![ScriptAddress::new_p2sh(AddressHashEnum::AddressHash(
self.data[2..22].into(),
))]),
ScriptType::PubKeyHash => {
let hash = H160::from_slice(&self.data[3..32]).map_err(|_| keys::Error::InvalidAddress)?;
Ok(vec![ScriptAddress::new_p2pkh(AddressHashEnum::AddressHash(hash))])
},
ScriptType::ScriptHash => {
let hash = H160::from_slice(&self.data[2..22]).map_err(|_| keys::Error::InvalidAddress)?;
Ok(vec![ScriptAddress::new_p2sh(AddressHashEnum::AddressHash(hash))])
},
ScriptType::Multisig => {
let mut addresses: Vec<ScriptAddress> = Vec::new();
let mut pc = 1;
Expand All @@ -448,12 +451,14 @@ impl Script {
Ok(addresses)
},
ScriptType::NullData => Ok(vec![]),
ScriptType::WitnessScript => Ok(vec![ScriptAddress::new_p2wsh(AddressHashEnum::WitnessScriptHash(
self.data[2..34].into(),
))]),
ScriptType::WitnessKey => Ok(vec![ScriptAddress::new_p2wpkh(AddressHashEnum::AddressHash(
self.data[2..22].into(),
))]),
ScriptType::WitnessScript => {
let hash = H256::from_slice(&self.data[2..34]).map_err(|_| keys::Error::WitnessHashMismatched)?;
Ok(vec![ScriptAddress::new_p2wsh(AddressHashEnum::WitnessScriptHash(hash))])
},
ScriptType::WitnessKey => {
let hash = H160::from_slice(&self.data[2..22]).map_err(|_| keys::Error::InvalidAddress)?;
Ok(vec![ScriptAddress::new_p2wpkh(AddressHashEnum::AddressHash(hash))])
},
ScriptType::CallSender => {
Ok(vec![]) // TODO
},
Expand Down
Loading