From: Matt Corallo <git@bluematt.me>
Date: Thu, 17 Jan 2019 22:36:49 +0000 (-0500)
Subject: Ensure Message always unwraps in fuzztarget
X-Git-Tag: v0.0.12~241^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=refs%2Fheads%2F2019-01-deps-bump;p=rust-lightning

Ensure Message always unwraps in fuzztarget

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.
---

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index ba86e6298..42d4caa36 100644
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -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));
 			}
diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index f3d51e4bb..c9cd59476 100644
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -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);
diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs
index 01634f9d5..8e70ce7ad 100644
--- a/src/ln/channelmonitor.rs
+++ b/src/ln/channelmonitor.rs
@@ -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)
 				}
diff --git a/src/ln/router.rs b/src/ln/router.rs
index dd98e188f..e25f9b223 100644
--- a/src/ln/router.rs
+++ b/src/ln/router.rs
@@ -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
index 000000000..508acf08d
--- /dev/null
+++ b/src/util/fuzz_wrappers.rs
@@ -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()
+				}
+			}
+		}
+	}
+}
diff --git a/src/util/mod.rs b/src/util/mod.rs
index c1b6c2300..e4170590d 100644
--- a/src/util/mod.rs
+++ b/src/util/mod.rs
@@ -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;