From: Matt Corallo <git@bluematt.me>
Date: Sat, 25 Aug 2018 19:03:59 +0000 (-0400)
Subject: Update error deserialization in compliance with BOLT #1
X-Git-Tag: v0.0.12~338^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=755b76bf8393e444a348ce97196e68964b852346;p=rust-lightning

Update error deserialization in compliance with BOLT #1
---

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_error_message_target.rs b/fuzz/fuzz_targets/msg_error_message_target.rs
new file mode 100644
index 000000000..fa021e231
--- /dev/null
+++ b/fuzz/fuzz_targets/msg_error_message_target.rs
@@ -0,0 +1,48 @@
+// This file is auto-generated by gen_target.sh based on msg_target_template.txt
+// To modify it, modify msg_target_template.txt and run gen_target.sh instead.
+
+extern crate lightning;
+
+use lightning::ln::msgs;
+use lightning::util::reset_rng_state;
+
+use lightning::ln::msgs::{MsgEncodable, MsgDecodable};
+
+#[inline]
+pub fn do_test(data: &[u8]) {
+	reset_rng_state();
+	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")]
+#[macro_use] extern crate afl;
+#[cfg(feature = "afl")]
+fn main() {
+	fuzz!(|data| {
+		do_test(data);
+	});
+}
+
+#[cfg(feature = "honggfuzz")]
+#[macro_use] extern crate honggfuzz;
+#[cfg(feature = "honggfuzz")]
+fn main() {
+	loop {
+		fuzz!(|data| {
+			do_test(data);
+		});
+	}
+}
+
+extern crate hex;
+#[cfg(test)]
+mod tests {
+	#[test]
+	fn duplicate_crash() {
+		super::do_test(&::hex::decode("00").unwrap());
+	}
+}
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/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs b/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs
deleted file mode 100644
index a69ded2ea..000000000
--- a/fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs
+++ /dev/null
@@ -1,46 +0,0 @@
-// This file is auto-generated by gen_target.sh based on msg_target_template.txt
-// To modify it, modify msg_target_template.txt and run gen_target.sh instead.
-
-extern crate lightning;
-
-use lightning::ln::msgs;
-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);
-}
-
-#[cfg(feature = "afl")]
-#[macro_use] extern crate afl;
-#[cfg(feature = "afl")]
-fn main() {
-	fuzz!(|data| {
-		do_test(data);
-	});
-}
-
-#[cfg(feature = "honggfuzz")]
-#[macro_use] extern crate honggfuzz;
-#[cfg(feature = "honggfuzz")]
-fn main() {
-	loop {
-		fuzz!(|data| {
-			do_test(data);
-		});
-	}
-}
-
-extern crate hex;
-#[cfg(test)]
-mod tests {
-	#[test]
-	fn duplicate_crash() {
-		super::do_test(&::hex::decode("00").unwrap());
-	}
-}
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),
 		};