Skip to content

Commit

Permalink
Ensure Message always unwraps in fuzztarget
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Jan 22, 2019
1 parent f109d13 commit 8678bda
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 28 deletions.
32 changes: 16 additions & 16 deletions src/ln/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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");
Expand All @@ -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))
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand All @@ -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)?;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -2558,15 +2558,15 @@ 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(_) => {},
Err(_e) => {
// 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");
},
};
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -3295,15 +3295,15 @@ 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);
for &(ref htlc, _) in remote_commitment_tx.2.iter() {
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));
}
Expand Down
8 changes: 4 additions & 4 deletions src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/ln/channelmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
},
Expand Down Expand Up @@ -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)
},
Expand Down Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions src/ln/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 17 additions & 0 deletions src/util/fuzz_wrappers.rs
View file
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
}
}
3 changes: 3 additions & 0 deletions src/util/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit 8678bda

Please sign in to comment.