]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Merge pull request #14 from TheBlueMatt/2018-03-fuzz-fixes-1
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 23 Mar 2018 17:15:47 +0000 (13:15 -0400)
committerGitHub <noreply@github.com>
Fri, 23 Mar 2018 17:15:47 +0000 (13:15 -0400)
Impl some message deserialization, initial fuzzing-found bug fixes

src/ln/channel.rs
src/ln/msgs.rs
src/ln/peer_channel_encryptor.rs

index ef73db8eb813b2dea164cc00a4b245924c1898a1..2b8b085a8e4f84979697d9df26fc7515fce2e112 100644 (file)
@@ -43,34 +43,33 @@ pub struct ChannelKeys {
 
 impl ChannelKeys {
        pub fn new_from_seed(seed: &[u8; 32]) -> Result<ChannelKeys, secp256k1::Error> {
-               let sha = Sha256::new();
                let mut prk = [0; 32];
-               hkdf_extract(sha, b"rust-lightning key gen salt", seed, &mut prk);
+               hkdf_extract(Sha256::new(), b"rust-lightning key gen salt", seed, &mut prk);
                let secp_ctx = Secp256k1::new();
 
                let mut okm = [0; 32];
-               hkdf_expand(sha, &prk, b"rust-lightning funding key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning funding key info", &mut okm);
                let funding_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning revocation base key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning revocation base key info", &mut okm);
                let revocation_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning payment base key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning payment base key info", &mut okm);
                let payment_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning delayed payment base key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning delayed payment base key info", &mut okm);
                let delayed_payment_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning htlc base key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning htlc base key info", &mut okm);
                let htlc_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning channel close key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning channel close key info", &mut okm);
                let channel_close_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning channel monitor claim key info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning channel monitor claim key info", &mut okm);
                let channel_monitor_claim_key = SecretKey::from_slice(&secp_ctx, &okm)?;
 
-               hkdf_expand(sha, &prk, b"rust-lightning local commitment seed info", &mut okm);
+               hkdf_expand(Sha256::new(), &prk, b"rust-lightning local commitment seed info", &mut okm);
 
                Ok(ChannelKeys {
                        funding_key: funding_key,
@@ -367,7 +366,9 @@ impl Channel {
                if msg.push_msat > (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
                        return Err(HandleError{err: "push_msat more than highest possible value", msg: None});
                }
-               //TODO Check if dust_limit is sane?
+               if msg.dust_limit_satoshis > 21000000 * 100000000 {
+                       return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
+               }
                if msg.max_htlc_value_in_flight_msat > msg.funding_satoshis * 1000 {
                        return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
                }
@@ -827,13 +828,15 @@ impl Channel {
 
        pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
                // Check sanity of message fields:
-               //TODO Check if dust_limit is sane?
                if !self.channel_outbound {
                        return Err(HandleError{err: "Got an accept_channel message from an inbound peer", msg: None});
                }
                if self.channel_state != ChannelState::OurInitSent as u32 {
                        return Err(HandleError{err: "Got an accept_channel message at a strange time", msg: None});
                }
+               if msg.dust_limit_satoshis > 21000000 * 100000000 {
+                       return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
+               }
                if msg.max_htlc_value_in_flight_msat > self.channel_value_satoshis * 1000 {
                        return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
                }
@@ -1013,8 +1016,10 @@ impl Channel {
                if htlc_inbound_value_msat + msg.amount_msat > Channel::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) {
                        return Err(HandleError{err: "Remote HTLC add would put them over their max HTLC value in flight", msg: None});
                }
-               // Check our_channel_reserve_satoshis:
-               if htlc_inbound_value_msat + htlc_outbound_value_msat + msg.amount_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
+               // Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
+               // the reserve_satoshis we told them to always have as direct payment so that they lose
+               // something if we punish them for broadcasting an old state).
+               if htlc_inbound_value_msat + htlc_outbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
                        return Err(HandleError{err: "Remote HTLC add would put them over their reserve value", msg: None});
                }
                if self.next_remote_htlc_id != msg.htlc_id {
@@ -1592,7 +1597,7 @@ impl Channel {
                        return Err(HandleError{err: "Cannot send value that would put us over our max HTLC value in flight", msg: None});
                }
                // Check their_channel_reserve_satoshis:
-               if htlc_outbound_value_msat + amount_msat > (self.channel_value_satoshis - self.their_channel_reserve_satoshis) * 1000 - htlc_inbound_value_msat {
+               if htlc_inbound_value_msat + htlc_outbound_value_msat + amount_msat + (self.channel_value_satoshis * 1000 - self.value_to_self_msat) > (self.channel_value_satoshis - self.their_channel_reserve_satoshis) * 1000 {
                        return Err(HandleError{err: "Cannot send value that would put us over our reserve value", msg: None});
                }
 
index 3cdd4c77cb0efbd198b2c1ef782186e488088a6f..97ed1e320f9350b4ac69fc04ca7a8ce49abfbb5b 100644 (file)
@@ -684,8 +684,20 @@ impl MsgEncodable for ClosingSigned {
 }
 
 impl MsgDecodable for UpdateAddHTLC {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+8+8+32+4+1+33+20*65+32 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut payment_hash = [0; 32];
+               payment_hash.copy_from_slice(&v[48..80]);
+               Ok(Self{
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       htlc_id: byte_utils::slice_to_be64(&v[32..40]),
+                       amount_msat: byte_utils::slice_to_be64(&v[40..48]),
+                       payment_hash,
+                       cltv_expiry: byte_utils::slice_to_be32(&v[80..84]),
+                       onion_routing_packet: OnionPacket::decode(&v[84..])?,
+               })
        }
 }
 impl MsgEncodable for UpdateAddHTLC {
@@ -695,8 +707,17 @@ impl MsgEncodable for UpdateAddHTLC {
 }
 
 impl MsgDecodable for UpdateFulfillHTLC {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+8+32 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut payment_preimage = [0; 32];
+               payment_preimage.copy_from_slice(&v[40..72]);
+               Ok(Self{
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       htlc_id: byte_utils::slice_to_be64(&v[32..40]),
+                       payment_preimage,
+               })
        }
 }
 impl MsgEncodable for UpdateFulfillHTLC {
@@ -706,8 +727,15 @@ impl MsgEncodable for UpdateFulfillHTLC {
 }
 
 impl MsgDecodable for UpdateFailHTLC {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+8 {
+                       return Err(DecodeError::WrongLength);
+               }
+               Ok(Self{
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       htlc_id: byte_utils::slice_to_be64(&v[32..40]),
+                       reason: OnionErrorPacket::decode(&v[40..])?,
+               })
        }
 }
 impl MsgEncodable for UpdateFailHTLC {
@@ -717,8 +745,18 @@ impl MsgEncodable for UpdateFailHTLC {
 }
 
 impl MsgDecodable for UpdateFailMalformedHTLC {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+8+32+2 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut sha256_of_onion = [0; 32];
+               sha256_of_onion.copy_from_slice(&v[40..72]);
+               Ok(Self{
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       htlc_id: byte_utils::slice_to_be64(&v[32..40]),
+                       sha256_of_onion,
+                       failure_code: byte_utils::slice_to_be16(&v[72..74]),
+               })
        }
 }
 impl MsgEncodable for UpdateFailMalformedHTLC {
@@ -728,8 +766,24 @@ impl MsgEncodable for UpdateFailMalformedHTLC {
 }
 
 impl MsgDecodable for CommitmentSigned {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+64+2 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let htlcs = byte_utils::slice_to_be16(&v[96..98]) as usize;
+               if v.len() < 32+64+2+htlcs*64 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut htlc_signatures = Vec::with_capacity(htlcs);
+               let secp_ctx = Secp256k1::without_caps();
+               for i in 0..htlcs {
+                       htlc_signatures.push(secp_signature!(&secp_ctx, &v[98+i*64..98+(i+1)*64]));
+               }
+               Ok(Self {
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       signature: secp_signature!(&secp_ctx, &v[32..96]),
+                       htlc_signatures,
+               })
        }
 }
 impl MsgEncodable for CommitmentSigned {
@@ -739,8 +793,18 @@ impl MsgEncodable for CommitmentSigned {
 }
 
 impl MsgDecodable for RevokeAndACK {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+32+33 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut per_commitment_secret = [0; 32];
+               per_commitment_secret.copy_from_slice(&v[32..64]);
+               let secp_ctx = Secp256k1::without_caps();
+               Ok(Self {
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       per_commitment_secret,
+                       next_per_commitment_point: secp_pubkey!(&secp_ctx, &v[64..97]),
+               })
        }
 }
 impl MsgEncodable for RevokeAndACK {
@@ -750,8 +814,14 @@ impl MsgEncodable for RevokeAndACK {
 }
 
 impl MsgDecodable for UpdateFee {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 32+4 {
+                       return Err(DecodeError::WrongLength);
+               }
+               Ok(Self {
+                       channel_id: deserialize(&v[0..32]).unwrap(),
+                       feerate_per_kw: byte_utils::slice_to_be32(&v[32..36]),
+               })
        }
 }
 impl MsgEncodable for UpdateFee {
@@ -954,8 +1024,21 @@ impl MsgEncodable for OnionHopData {
 }
 
 impl MsgDecodable for OnionPacket {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 1+33+20*65+32 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let mut hop_data = [0; 20*65];
+               hop_data.copy_from_slice(&v[34..1334]);
+               let mut hmac = [0; 32];
+               hmac.copy_from_slice(&v[1334..1366]);
+               let secp_ctx = Secp256k1::without_caps();
+               Ok(Self {
+                       version: v[0],
+                       public_key: secp_pubkey!(&secp_ctx, &v[1..34]),
+                       hop_data,
+                       hmac,
+               })
        }
 }
 impl MsgEncodable for OnionPacket {
@@ -987,8 +1070,17 @@ impl MsgEncodable for DecodedOnionErrorPacket {
 }
 
 impl MsgDecodable for OnionErrorPacket {
-       fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
-               unimplemented!();
+       fn decode(v: &[u8]) -> Result<Self, DecodeError> {
+               if v.len() < 2 {
+                       return Err(DecodeError::WrongLength);
+               }
+               let len = byte_utils::slice_to_be16(&v[0..2]) as usize;
+               if v.len() < 2 + len {
+                       return Err(DecodeError::WrongLength);
+               }
+               Ok(Self {
+                       data: v[2..len+2].to_vec(),
+               })
        }
 }
 impl MsgEncodable for OnionErrorPacket {
index 2de38e68f8a0dd7ff297cd2e63a23fc43dde559c..e22d8bd3574969cdee2ff1470a1207b5f4607480 100644 (file)
@@ -157,12 +157,11 @@ impl PeerChannelEncryptor {
 
        #[inline]
        fn hkdf(state: &mut BidirectionalNoiseState, ss: SharedSecret) -> [u8; 32] {
-               let sha = Sha256::new();
                let mut hkdf = [0; 64];
                {
                        let mut prk = [0; 32];
-                       hkdf_extract(sha, &state.ck, &ss[..], &mut prk);
-                       hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
+                       hkdf_extract(Sha256::new(), &state.ck, &ss[..], &mut prk);
+                       hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);
                }
                state.ck.copy_from_slice(&hkdf[0..32]);
                let mut res = [0; 32];
@@ -313,10 +312,9 @@ impl PeerChannelEncryptor {
 
                                                PeerChannelEncryptor::encrypt_with_ad(&mut res[50..], 0, &temp_k, &bidirectional_state.h, &[0; 0]);
 
-                                               sha.reset();
                                                let mut prk = [0; 32];
-                                               hkdf_extract(sha, &bidirectional_state.ck, &[0; 0], &mut prk);
-                                               hkdf_expand(sha, &prk, &[0;0], &mut final_hkdf);
+                                               hkdf_extract(Sha256::new(), &bidirectional_state.ck, &[0; 0], &mut prk);
+                                               hkdf_expand(Sha256::new(), &prk, &[0;0], &mut final_hkdf);
                                                ck = bidirectional_state.ck.clone();
                                                res
                                        },
@@ -375,10 +373,9 @@ impl PeerChannelEncryptor {
 
                                                PeerChannelEncryptor::decrypt_with_ad(&mut [0; 0], 0, &temp_k, &bidirectional_state.h, &act_three[50..])?;
 
-                                               sha.reset();
                                                let mut prk = [0; 32];
-                                               hkdf_extract(sha, &bidirectional_state.ck, &[0; 0], &mut prk);
-                                               hkdf_expand(sha, &prk, &[0;0], &mut final_hkdf);
+                                               hkdf_extract(Sha256::new(), &bidirectional_state.ck, &[0; 0], &mut prk);
+                                               hkdf_expand(Sha256::new(), &prk, &[0;0], &mut final_hkdf);
                                                ck = bidirectional_state.ck.clone();
                                        },
                                        _ => panic!("Wrong direction for act"),
@@ -416,11 +413,10 @@ impl PeerChannelEncryptor {
                match self.noise_state {
                        NoiseState::Finished { ref mut sk, ref mut sn, ref mut sck, rk: _, rn: _, rck: _ } => {
                                if *sn >= 1000 {
-                                       let mut sha = Sha256::new();
                                        let mut prk = [0; 32];
-                                       hkdf_extract(sha, sck, sk, &mut prk);
+                                       hkdf_extract(Sha256::new(), sck, sk, &mut prk);
                                        let mut hkdf = [0; 64];
-                                       hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
+                                       hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);
 
                                        sck[..].copy_from_slice(&hkdf[0..32]);
                                        sk[..].copy_from_slice(&hkdf[32..]);
@@ -447,11 +443,10 @@ impl PeerChannelEncryptor {
                match self.noise_state {
                        NoiseState::Finished { sk: _, sn: _, sck: _, ref mut rk, ref mut rn, ref mut rck } => {
                                if *rn >= 1000 {
-                                       let mut sha = Sha256::new();
                                        let mut prk = [0; 32];
-                                       hkdf_extract(sha, rck, rk, &mut prk);
+                                       hkdf_extract(Sha256::new(), rck, rk, &mut prk);
                                        let mut hkdf = [0; 64];
-                                       hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
+                                       hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);
 
                                        rck[..].copy_from_slice(&hkdf[0..32]);
                                        rk[..].copy_from_slice(&hkdf[32..]);
@@ -752,7 +747,7 @@ mod tests {
                        let res = outbound_peer.encrypt_message(&msg);
                        assert_eq!(res.len(), 5 + 2*16 + 2);
 
-                       let mut len_header = res[0..2+16].to_vec();
+                       let len_header = res[0..2+16].to_vec();
                        assert_eq!(inbound_peer.decrypt_length_header(&len_header[..]).unwrap() as usize, msg.len());
                        assert_eq!(inbound_peer.decrypt_message(&res[2+16..]).unwrap()[..], msg[..]);