diff --git a/libwallet/src/slate_versions/ser.rs b/libwallet/src/slate_versions/ser.rs index 58f866a..8e3510c 100644 --- a/libwallet/src/slate_versions/ser.rs +++ b/libwallet/src/slate_versions/ser.rs @@ -386,8 +386,8 @@ pub mod dalek_sig_serde { String::deserialize(deserializer) .and_then(|string| from_hex(&string).map_err(|err| Error::custom(err.to_string()))) .and_then(|bytes: Vec| { - let mut b = [0u8; 64]; - b.copy_from_slice(&bytes[0..64]); + let b = <[u8; 64]>::try_from(bytes.as_slice()) + .map_err(|_| Error::custom("invalid signature length"))?; DalekSignature::try_from(b).map_err(|err| Error::custom(err.to_string())) }) } @@ -422,8 +422,8 @@ pub mod option_dalek_sig_serde { Some(string) => from_hex(&string) .map_err(|err| Error::custom(err.to_string())) .and_then(|bytes: Vec| { - let mut b = [0u8; 64]; - b.copy_from_slice(&bytes[0..64]); + let b = <[u8; 64]>::try_from(bytes.as_slice()) + .map_err(|_| Error::custom("invalid signature length"))?; DalekSignature::try_from(b) .map(Some) .map_err(|err| Error::custom(err.to_string())) @@ -461,8 +461,8 @@ pub mod option_dalek_sig_base64 { Some(string) => base64::decode(&string) .map_err(|err| Error::custom(err.to_string())) .and_then(|bytes: Vec| { - let mut b = [0u8; 64]; - b.copy_from_slice(&bytes[0..64]); + let b = <[u8; 64]>::try_from(bytes.as_slice()) + .map_err(|_| Error::custom("invalid signature length"))?; DalekSignature::try_from(b) .map(Some) .map_err(|err| Error::custom(err.to_string())) diff --git a/libwallet/src/slate_versions/v4_bin.rs b/libwallet/src/slate_versions/v4_bin.rs index 68f5190..455af3f 100644 --- a/libwallet/src/slate_versions/v4_bin.rs +++ b/libwallet/src/slate_versions/v4_bin.rs @@ -345,11 +345,16 @@ impl<'a> Writeable for ProofWrapRef<'a> { impl Readable for ProofWrap { fn read(reader: &mut R) -> Result { - let saddr = DalekPublicKey::from_bytes(&reader.read_fixed_bytes(32)?).unwrap(); - let raddr = DalekPublicKey::from_bytes(&reader.read_fixed_bytes(32)?).unwrap(); + let saddr = DalekPublicKey::from_bytes(&reader.read_fixed_bytes(32)?) + .map_err(|_| grin_ser::Error::CorruptedData)?; + let raddr = DalekPublicKey::from_bytes(&reader.read_fixed_bytes(32)?) + .map_err(|_| grin_ser::Error::CorruptedData)?; let rsig = match reader.read_u8()? { 0 => None, - 1 | _ => Some(DalekSignature::try_from(&reader.read_fixed_bytes(64)?[..]).unwrap()), + 1 | _ => Some( + DalekSignature::try_from(&reader.read_fixed_bytes(64)?[..]) + .map_err(|_| grin_ser::Error::CorruptedData)?, + ), }; Ok(ProofWrap(PaymentInfoV4 { saddr, raddr, rsig })) } diff --git a/libwallet/src/slatepack/armor.rs b/libwallet/src/slatepack/armor.rs index eacdf32..c8be299 100644 --- a/libwallet/src/slatepack/armor.rs +++ b/libwallet/src/slatepack/armor.rs @@ -74,7 +74,10 @@ impl SlatepackArmor { // Get the length of the header let header_len = header_bytes.len() + 1; // Skip the length of the header to read for the payload until the next period - let payload_bytes = armor_bytes[header_len as usize..] + let payload_source = armor_bytes + .get(header_len..) + .ok_or_else(|| Error::InvalidSlatepackData("Bad armor header".to_string()))?; + let payload_bytes = payload_source .iter() .take_while(|byte| **byte != b'.') .cloned() @@ -82,8 +85,14 @@ impl SlatepackArmor { // Get length of the payload to check the footer framing let payload_len = payload_bytes.len(); // Get footer bytes and verify them - let consumed_bytes = header_len + payload_len + 1; - let footer_bytes = armor_bytes[consumed_bytes as usize..] + let consumed_bytes = header_len + .checked_add(payload_len) + .and_then(|v| v.checked_add(1)) + .ok_or_else(|| Error::InvalidSlatepackData("Bad armor footer".to_string()))?; + let footer_source = armor_bytes + .get(consumed_bytes..) + .ok_or_else(|| Error::InvalidSlatepackData("Bad armor footer".to_string()))?; + let footer_bytes = footer_source .iter() .take_while(|byte| **byte != b'.') .cloned() @@ -99,8 +108,10 @@ impl SlatepackArmor { let base_decode = bs58::decode(&clean_payload) .into_vec() .map_err(|_| Error::SlatepackDeser("Bad bytes".into()))?; - let error_code = &base_decode[0..4]; - let slatepack_bytes = &base_decode[4..]; + if base_decode.len() < 4 { + return Err(Error::SlatepackDeser("Payload too short".into())); + } + let (error_code, slatepack_bytes) = base_decode.split_at(4); // Make sure the error check code is valid for the slate data error_check(error_code, slatepack_bytes)?; // Return slate as binary or string diff --git a/libwallet/src/slatepack/types.rs b/libwallet/src/slatepack/types.rs index fe08c93..fc12f49 100644 --- a/libwallet/src/slatepack/types.rs +++ b/libwallet/src/slatepack/types.rs @@ -208,10 +208,23 @@ impl Slatepack { reader.read_to_end(&mut decrypted)?; // Parse encrypted metadata from payload, first 4 bytes of decrypted payload // will be encrypted metadata length + if decrypted.len() < 4 { + return Err(Error::SlatepackDeser( + "Encrypted payload missing metadata length".into(), + )); + } let mut len_bytes = [0u8; 4]; len_bytes.copy_from_slice(&decrypted[0..4]); let meta_len = Cursor::new(len_bytes).read_u32::()?; - self.payload = decrypted.split_off(meta_len as usize + 4); + let payload_start = (meta_len as usize) + .checked_add(4) + .ok_or_else(|| Error::SlatepackDeser("Encrypted metadata length overflow".into()))?; + if payload_start > decrypted.len() { + return Err(Error::SlatepackDeser( + "Encrypted metadata length exceeds payload".into(), + )); + } + self.payload = decrypted.split_off(payload_start); let meta = byte_ser::from_bytes::(&decrypted) .map_err(|_| Error::SlatepackSer)? .0; @@ -848,3 +861,53 @@ fn slatepack_encrypted_meta_future() -> Result<(), Error> { Ok(()) } + +#[cfg(test)] +fn slatepack_test_decryption_key() -> (edSecretKey, SlatepackAddress) { + use ed25519_dalek::PublicKey as edDalekPublicKey; + use rand::{thread_rng, Rng}; + + let sec_key_bytes: [u8; 32] = thread_rng().gen(); + let ed_sec_key = edSecretKey::from_bytes(&sec_key_bytes).unwrap(); + let ed_pub_key = edDalekPublicKey::from(&ed_sec_key); + let addr = SlatepackAddress::new(&ed_pub_key); + + (ed_sec_key, addr) +} + +#[cfg(test)] +fn encrypt_plaintext_to_slatepack_recipient( + recipient: &SlatepackAddress, + plaintext: &[u8], +) -> Result, Error> { + let recp_key: age::x25519::Recipient = recipient.to_age_pubkey_str()?.parse()?; + let keys = vec![Box::new(recp_key) as Box]; + let encryptor = age::Encryptor::with_recipients(keys); + let mut encrypted = vec![]; + let mut writer = encryptor.wrap_output(&mut encrypted)?; + writer.write_all(plaintext)?; + writer.finish()?; + Ok(encrypted) +} + +#[test] +fn slatepack_decrypt_rejects_malformed_plaintexts() -> Result<(), Error> { + use crate::grin_core::global; + + global::set_local_chain_type(global::ChainTypes::AutomatedTesting); + let (ed_sec_key, addr) = slatepack_test_decryption_key(); + + for plaintext in vec![vec![], vec![0], vec![0, 0, 0], vec![0xff; 4]] { + let mut slatepack = Slatepack { + mode: 1, + payload: encrypt_plaintext_to_slatepack_recipient(&addr, &plaintext)?, + ..Slatepack::default() + }; + assert!(matches!( + slatepack.try_decrypt_payload(Some(&ed_sec_key)), + Err(Error::SlatepackDeser(_)) + )); + } + + Ok(()) +}