Update error deserialization in compliance with BOLT #1

diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
index 4394fd7964d646debbec2b35ec2ff95d65b5ec88..85ba3d11f3b08c6103e45017b8e75ca23e87949e 100644 (file)
--- 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 (file)
index 0000000..fa021e2
--- /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 37eac67a720e8304c0db54859ccd49651807a292..4a5dc2fc5565d9c25739c97b01faf512c5f74e72 100755 (executable)
--- 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 (file)
index a69ded2..0000000
--- 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 4500ee2f89f48bee0290257c3c49411169cc0c07..44d71bd332938bc3959f3cf768bfebe5bffcbdcb 100644 (file)
--- 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),
                };