From 755b76bf8393e444a348ce97196e68964b852346 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 25 Aug 2018 15:03:59 -0400 Subject: [PATCH] Update error deserialization in compliance with BOLT #1 --- fuzz/Cargo.toml | 8 ++++---- .../{msg_targets => }/msg_error_message_target.rs | 8 +++++--- fuzz/fuzz_targets/msg_targets/gen_target.sh | 2 +- src/ln/msgs.rs | 10 ++++------ 4 files changed, 14 insertions(+), 14 deletions(-) rename fuzz/fuzz_targets/{msg_targets => }/msg_error_message_target.rs (82%) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 4394fd796..85ba3d11f 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -60,6 +60,10 @@ path = "fuzz_targets/msg_ping_target.rs" name = "msg_pong_target" path = "fuzz_targets/msg_pong_target.rs" +[[bin]] +name = "msg_error_message_target" +path = "fuzz_targets/msg_error_message_target.rs" + [[bin]] name = "msg_accept_channel_target" path = "fuzz_targets/msg_targets/msg_accept_channel_target.rs" @@ -119,7 +123,3 @@ path = "fuzz_targets/msg_targets/msg_update_fail_htlc_target.rs" [[bin]] name = "msg_channel_reestablish_target" path = "fuzz_targets/msg_targets/msg_channel_reestablish_target.rs" - -[[bin]] -name = "msg_error_message_target" -path = "fuzz_targets/msg_targets/msg_error_message_target.rs" diff --git a/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs b/fuzz/fuzz_targets/msg_error_message_target.rs similarity index 82% rename from fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs rename to fuzz/fuzz_targets/msg_error_message_target.rs index a69ded2ea..fa021e231 100644 --- a/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs +++ b/fuzz/fuzz_targets/msg_error_message_target.rs @@ -8,12 +8,14 @@ use lightning::util::reset_rng_state; use lightning::ln::msgs::{MsgEncodable, MsgDecodable}; -mod utils; - #[inline] pub fn do_test(data: &[u8]) { reset_rng_state(); - test_msg!(msgs::ErrorMessage, data); + if let Ok(msg) = msgs::ErrorMessage::decode(data){ + let enc = msg.encode(); + assert_eq!(&data[0..32], &enc[0..32]); + assert_eq!(&data[34..enc.len()], &enc[34..]); + } } #[cfg(feature = "afl")] diff --git a/fuzz/fuzz_targets/msg_targets/gen_target.sh b/fuzz/fuzz_targets/msg_targets/gen_target.sh index 37eac67a7..4a5dc2fc5 100755 --- a/fuzz/fuzz_targets/msg_targets/gen_target.sh +++ b/fuzz/fuzz_targets/msg_targets/gen_target.sh @@ -1,4 +1,4 @@ -for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateAddHTLC UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish ErrorMessage; do +for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateAddHTLC UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish; do tn=$(echo $target | sed 's/\([a-z0-9]\)\([A-Z]\)/\1_\2/g') fn=msg_$(echo $tn | tr '[:upper:]' '[:lower:]')_target.rs cat msg_target_template.txt | sed s/MSG_TARGET/$target/ > $fn diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs index 4500ee2f8..44d71bd33 100644 --- a/src/ln/msgs.rs +++ b/src/ln/msgs.rs @@ -5,7 +5,7 @@ use bitcoin::network::serialize::{deserialize,serialize}; use bitcoin::blockdata::script::Script; use std::error::Error; -use std::fmt; +use std::{cmp, fmt}; use std::result::Result; use util::{byte_utils, internal_traits, events}; @@ -1626,11 +1626,9 @@ impl MsgDecodable for ErrorMessage { if v.len() < 34 { return Err(DecodeError::ShortRead); } - let len = byte_utils::slice_to_be16(&v[32..34]); - if v.len() < 34 + len as usize { - return Err(DecodeError::ShortRead); - } - let data = match String::from_utf8(v[34..34 + len as usize].to_vec()) { + // Unlike most messages, BOLT 1 requires we truncate our read if the value is out of range + let len = cmp::min(byte_utils::slice_to_be16(&v[32..34]) as usize, v.len() - 34); + let data = match String::from_utf8(v[34..34 + len].to_vec()) { Ok(s) => s, Err(_) => return Err(DecodeError::BadText), }; -- 2.39.5