]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Ensure Message always unwraps in fuzztarget 2019-01-deps-bump
authorMatt Corallo <git@bluematt.me>
Thu, 17 Jan 2019 22:36:49 +0000 (17:36 -0500)
committerMatt Corallo <git@bluematt.me>
Tue, 22 Jan 2019 18:49:15 +0000 (13:49 -0500)
Hashes cant be all-0s, so we can normally unwrap, but fuzztarget
can generate all-0 hashes, so we have to handle it and swap for
something else.

src/ln/channel.rs
src/ln/channelmanager.rs
src/ln/channelmonitor.rs
src/ln/router.rs
src/util/fuzz_wrappers.rs [new file with mode: 0644]
src/util/mod.rs

index ba86e629825ac316e4cf1beeb5946fc7277fcba8..42d4caa36a84da209bb59b4cc5807493570ab1b3 100644 (file)
@@ -11,7 +11,7 @@ use bitcoin_hashes::sha256::Hash as Sha256;
 use bitcoin_hashes::hash160::Hash as Hash160;
 
 use secp256k1::key::{PublicKey,SecretKey};
-use secp256k1::{Secp256k1,Message,Signature};
+use secp256k1::{Secp256k1,Signature};
 use secp256k1;
 
 use ln::msgs;
@@ -1067,7 +1067,7 @@ impl Channel {
 
                let funding_redeemscript = self.get_funding_redeemscript();
 
-               let sighash = Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+               let sighash = hash_to_message!(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
                let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key);
 
                tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
@@ -1104,7 +1104,7 @@ impl Channel {
                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
 
                let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
-               let sighash = Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+               let sighash = hash_to_message!(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
                let is_local_tx = PublicKey::from_secret_key(&self.secp_ctx, &our_htlc_key) == keys.a_htlc_key;
                Ok((htlc_redeemscript, self.secp_ctx.sign(&sighash, &our_htlc_key), is_local_tx))
        }
@@ -1408,7 +1408,7 @@ impl Channel {
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
                let mut local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
-               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
                // They sign the "local" commitment transaction...
                secp_check!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey.unwrap()), "Invalid funding_created signature from peer");
@@ -1418,7 +1418,7 @@ impl Channel {
 
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
-               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
                // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
                Ok((remote_initial_commitment_tx, local_initial_commitment_tx, self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key), local_keys))
@@ -1487,7 +1487,7 @@ impl Channel {
 
                let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
                let mut local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
-               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
                // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
                secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid funding_signed signature from peer");
@@ -1699,7 +1699,7 @@ impl Channel {
                        (commitment_tx.0, commitment_tx.1, htlcs_cloned)
                };
                let local_commitment_txid = local_commitment_tx.0.txid();
-               let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
                secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid commitment tx signature from peer");
 
                //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
@@ -1725,7 +1725,7 @@ impl Channel {
                        if let Some(_) = htlc.transaction_output_index {
                                let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
-                               let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                               let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
                                secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
                                let htlc_sig = if htlc.offered {
                                        let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
@@ -2456,7 +2456,7 @@ impl Channel {
 
                let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                let funding_redeemscript = self.get_funding_redeemscript();
-               let sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+               let sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
 
                self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis));
                Some(msgs::ClosingSigned {
@@ -2558,7 +2558,7 @@ impl Channel {
                if used_total_fee != msg.fee_satoshis {
                        return Err(ChannelError::Close("Remote sent us a closing_signed with a fee greater than the value they can claim"));
                }
-               let mut sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+               let mut sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
 
                match self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey.unwrap()) {
                        Ok(_) => {},
@@ -2566,7 +2566,7 @@ impl Channel {
                                // The remote end may have decided to revoke their output due to inconsistent dust
                                // limits, so check for that case by re-checking the signature here.
                                closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0;
-                               sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+                               sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
                                secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid closing tx signature from peer");
                        },
                };
@@ -2584,7 +2584,7 @@ impl Channel {
                        ($new_feerate: expr) => {
                                let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap());
                                let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false);
-                               sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
+                               sighash = hash_to_message!(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]);
                                let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key);
                                self.last_sent_closing_fee = Some(($new_feerate, used_total_fee));
                                return Ok((Some(msgs::ClosingSigned {
@@ -2993,7 +2993,7 @@ impl Channel {
 
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
-               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]);
 
                // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
                Ok((self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key), remote_initial_commitment_tx))
@@ -3080,7 +3080,7 @@ impl Channel {
                        excess_data: Vec::new(),
                };
 
-               let msghash = Message::from_slice(&Sha256dHash::from_data(&msg.encode()[..])[..]).unwrap();
+               let msghash = hash_to_message!(&Sha256dHash::from_data(&msg.encode()[..])[..]);
                let sig = self.secp_ctx.sign(&msghash, &self.local_keys.funding_key);
 
                Ok((msg, sig))
@@ -3295,7 +3295,7 @@ impl Channel {
                let remote_keys = self.build_remote_transaction_keys()?;
                let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true, feerate_per_kw);
                let remote_commitment_txid = remote_commitment_tx.0.txid();
-               let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
+               let remote_sighash = hash_to_message!(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
                let our_sig = self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key);
 
                let mut htlc_sigs = Vec::with_capacity(remote_commitment_tx.1);
@@ -3303,7 +3303,7 @@ impl Channel {
                        if let Some(_) = htlc.transaction_output_index {
                                let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
-                               let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                               let htlc_sighash = hash_to_message!(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]);
                                let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
                                htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
                        }
index f3d51e4bb1d3d841cd8049177a460d96b13e78fd..c9cd5947669529575c3fa8738e829dfa7aa731bc 100644 (file)
@@ -20,7 +20,7 @@ use bitcoin_hashes::sha256::Hash as Sha256;
 use bitcoin_hashes::cmp::fixed_time_eq;
 
 use secp256k1::key::{SecretKey,PublicKey};
-use secp256k1::{Secp256k1,Message};
+use secp256k1::Secp256k1;
 use secp256k1::ecdh::SharedSecret;
 use secp256k1;
 
@@ -937,7 +937,7 @@ impl ChannelManager {
                };
 
                let msg_hash = Sha256dHash::from_data(&unsigned.encode()[..]);
-               let sig = self.secp_ctx.sign(&Message::from_slice(&msg_hash[..]).unwrap(), &self.our_network_key);
+               let sig = self.secp_ctx.sign(&hash_to_message!(&msg_hash[..]), &self.our_network_key);
 
                Ok(msgs::ChannelUpdate {
                        signature: sig,
@@ -1130,7 +1130,7 @@ impl ChannelManager {
                        Ok(res) => res,
                        Err(_) => return None, // Only in case of state precondition violations eg channel is closing
                };
-               let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
+               let msghash = hash_to_message!(&Sha256dHash::from_data(&announcement.encode()[..])[..]);
                let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
 
                Some(msgs::AnnouncementSignatures {
@@ -2101,7 +2101,7 @@ impl ChannelManager {
                                        try_chan_entry!(self, chan.get_mut().get_channel_announcement(our_node_id.clone(), self.genesis_hash.clone()), channel_state, chan);
 
                                let were_node_one = announcement.node_id_1 == our_node_id;
-                               let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
+                               let msghash = hash_to_message!(&Sha256dHash::from_data(&announcement.encode()[..])[..]);
                                if self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }).is_err() ||
                                                self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }).is_err() {
                                        try_chan_entry!(self, Err(ChannelError::Close("Bad announcement_signatures node_signature")), channel_state, chan);
index 01634f9d5f66c21249ea528af7141be71926ae4b..8e70ce7adba6b04b77fa369f549b15ac350733d5 100644 (file)
@@ -24,7 +24,7 @@ use bitcoin_hashes::Hash;
 use bitcoin_hashes::sha256::Hash as Sha256;
 use bitcoin_hashes::hash160::Hash as Hash160;
 
-use secp256k1::{Secp256k1,Message,Signature};
+use secp256k1::{Secp256k1,Signature};
 use secp256k1::key::{SecretKey,PublicKey};
 use secp256k1;
 
@@ -1105,7 +1105,7 @@ impl ChannelMonitor {
                                                                        let htlc = &per_commitment_option.unwrap()[$htlc_idx.unwrap()].0;
                                                                        chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey)
                                                                };
-                                                               let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
+                                                               let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
                                                                let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
                                                                (self.secp_ctx.sign(&sighash, &revocation_key), redeemscript)
                                                        },
@@ -1327,7 +1327,7 @@ impl ChannelMonitor {
                                                                        Storage::Local { ref htlc_base_key, .. } => {
                                                                                let htlc = &per_commitment_option.unwrap()[$input.sequence as usize].0;
                                                                                let redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
-                                                                               let sighash = ignore_error!(Message::from_slice(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]));
+                                                                               let sighash = hash_to_message!(&$sighash_parts.sighash_all(&$input, &redeemscript, $amount)[..]);
                                                                                let htlc_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key));
                                                                                (self.secp_ctx.sign(&sighash, &htlc_key), redeemscript)
                                                                        },
@@ -1512,7 +1512,7 @@ impl ChannelMonitor {
 
                        let sig = match self.key_storage {
                                Storage::Local { ref revocation_base_key, .. } => {
-                                       let sighash = ignore_error!(Message::from_slice(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]));
+                                       let sighash = hash_to_message!(&sighash_parts.sighash_all(&spend_tx.input[0], &redeemscript, amount)[..]);
                                        let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key));
                                        self.secp_ctx.sign(&sighash, &revocation_key)
                                }
index dd98e188f5b3cda71fed307f87f0cc888ad2512a..e25f9b223c05b0b9fc7216c7852191b8493c2033 100644 (file)
@@ -4,7 +4,7 @@
 //! interrogate it to get routes for your own payments.
 
 use secp256k1::key::PublicKey;
-use secp256k1::{Secp256k1,Message};
+use secp256k1::Secp256k1;
 use secp256k1;
 
 use bitcoin::util::hash::Sha256dHash;
@@ -239,7 +239,7 @@ macro_rules! secp_verify_sig {
 
 impl RoutingMessageHandler for Router {
        fn handle_node_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<bool, HandleError> {
-               let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
+               let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
                secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
 
                if msg.contents.features.requires_unknown_bits() {
@@ -272,7 +272,7 @@ impl RoutingMessageHandler for Router {
                        return Err(HandleError{err: "Channel announcement node had a channel with itself", action: Some(ErrorAction::IgnoreError)});
                }
 
-               let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
+               let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
                secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_1, &msg.contents.node_id_1);
                secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.node_signature_2, &msg.contents.node_id_2);
                secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
@@ -448,7 +448,7 @@ impl RoutingMessageHandler for Router {
                                                };
                                        }
                                }
-                               let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
+                               let msg_hash = hash_to_message!(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]);
                                if msg.contents.flags & 1 == 1 {
                                        dest_node_id = channel.one_to_two.src_node_id.clone();
                                        secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &channel.two_to_one.src_node_id);
diff --git a/src/util/fuzz_wrappers.rs b/src/util/fuzz_wrappers.rs
new file mode 100644 (file)
index 0000000..508acf0
--- /dev/null
@@ -0,0 +1,17 @@
+macro_rules! hash_to_message {
+       ($slice: expr) => {
+               {
+                       #[cfg(not(feature = "fuzztarget"))]
+                       {
+                               ::secp256k1::Message::from_slice($slice).unwrap()
+                       }
+                       #[cfg(feature = "fuzztarget")]
+                       {
+                               match ::secp256k1::Message::from_slice($slice) {
+                                       Ok(msg) => msg,
+                                       Err(_) => ::secp256k1::Message::from_slice(&[1; 32]).unwrap()
+                               }
+                       }
+               }
+       }
+}
index c1b6c230064134d7e19b05f77e0b16875cc040c7..e4170590d1003045deb0cd2567ff70e71aa461a9 100644 (file)
@@ -27,3 +27,6 @@ pub use self::rng::{reset_rng_state, fill_bytes};
 
 #[cfg(test)]
 pub(crate) mod test_utils;
+
+#[macro_use]
+pub(crate) mod fuzz_wrappers;