From: Matt Corallo <git@bluematt.me>
Date: Wed, 25 Jul 2018 19:27:19 +0000 (-0400)
Subject: Update NodeAnnouncement addr deserialization to check addr len.
X-Git-Tag: v0.0.12~368^2~3
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=66023b7886b821767b732a1354a5f486ff8da167;p=rust-lightning

Update NodeAnnouncement addr deserialization to check addr len.

This more aggressively checks the message contents are correct
before returning WrongLength so existing fuzz setup has an easier
time.
---

diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs
index 73ea2c1d8..843c95ab7 100644
--- a/fuzz/fuzz_targets/channel_target.rs
+++ b/fuzz/fuzz_targets/channel_target.rs
@@ -120,6 +120,7 @@ pub fn do_test(data: &[u8]) {
 					msgs::DecodeError::BadSignature => return,
 					msgs::DecodeError::BadText => return,
 					msgs::DecodeError::ExtraAddressesPerType => return,
+					msgs::DecodeError::BadLengthDescriptor => return,
 					msgs::DecodeError::ShortRead => panic!("We picked the length..."),
 				}
 			}
@@ -141,6 +142,7 @@ pub fn do_test(data: &[u8]) {
 						msgs::DecodeError::BadSignature => return,
 						msgs::DecodeError::BadText => return,
 						msgs::DecodeError::ExtraAddressesPerType => return,
+						msgs::DecodeError::BadLengthDescriptor => return,
 						msgs::DecodeError::ShortRead => panic!("We picked the length..."),
 					}
 				}
diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs
index 415af4c4d..7f502530a 100644
--- a/src/ln/msgs.rs
+++ b/src/ln/msgs.rs
@@ -37,6 +37,9 @@ pub enum DecodeError {
 	ShortRead,
 	/// node_announcement included more than one address of a given type!
 	ExtraAddressesPerType,
+	/// A length descriptor in the packet didn't describe the later data correctly
+	/// (currently only generated in node_announcement)
+	BadLengthDescriptor,
 }
 pub trait MsgDecodable: Sized {
 	fn decode(v: &[u8]) -> Result<Self, DecodeError>;
@@ -502,6 +505,7 @@ impl Error for DecodeError {
 			DecodeError::BadText => "Invalid text in packet",
 			DecodeError::ShortRead => "Packet extended beyond the provided bytes",
 			DecodeError::ExtraAddressesPerType => "More than one address of a single type",
+			DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly",
 		}
 	}
 }
@@ -1204,20 +1208,21 @@ impl MsgDecodable for UnsignedNodeAnnouncement {
 		if v.len() < start + 74 + addrlen {
 			return Err(DecodeError::ShortRead);
 		}
+		let addr_read_limit = start + 74 + addrlen;
 
 		let mut addresses = Vec::with_capacity(4);
 		let mut read_pos = start + 74;
 		loop {
-			if v.len() <= read_pos { break; }
+			if addr_read_limit <= read_pos { break; }
 			match v[read_pos] {
 				0 => { read_pos += 1; },
 				1 => {
-					if v.len() < read_pos + 1 + 6 {
-						return Err(DecodeError::ShortRead);
-					}
 					if addresses.len() > 0 {
 						return Err(DecodeError::ExtraAddressesPerType);
 					}
+					if addr_read_limit < read_pos + 1 + 6 {
+						return Err(DecodeError::BadLengthDescriptor);
+					}
 					let mut addr = [0; 4];
 					addr.copy_from_slice(&v[read_pos + 1..read_pos + 5]);
 					addresses.push(NetAddress::IPv4 {
@@ -1227,12 +1232,12 @@ impl MsgDecodable for UnsignedNodeAnnouncement {
 					read_pos += 1 + 6;
 				},
 				2 => {
-					if v.len() < read_pos + 1 + 18 {
-						return Err(DecodeError::ShortRead);
-					}
 					if addresses.len() > 1 || (addresses.len() == 1 && addresses[0].get_id() != 1) {
 						return Err(DecodeError::ExtraAddressesPerType);
 					}
+					if addr_read_limit < read_pos + 1 + 18 {
+						return Err(DecodeError::BadLengthDescriptor);
+					}
 					let mut addr = [0; 16];
 					addr.copy_from_slice(&v[read_pos + 1..read_pos + 17]);
 					addresses.push(NetAddress::IPv6 {
@@ -1242,12 +1247,12 @@ impl MsgDecodable for UnsignedNodeAnnouncement {
 					read_pos += 1 + 18;
 				},
 				3 => {
-					if v.len() < read_pos + 1 + 12 {
-						return Err(DecodeError::ShortRead);
-					}
 					if addresses.len() > 2 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 2) {
 						return Err(DecodeError::ExtraAddressesPerType);
 					}
+					if addr_read_limit < read_pos + 1 + 12 {
+						return Err(DecodeError::BadLengthDescriptor);
+					}
 					let mut addr = [0; 10];
 					addr.copy_from_slice(&v[read_pos + 1..read_pos + 11]);
 					addresses.push(NetAddress::OnionV2 {
@@ -1257,12 +1262,12 @@ impl MsgDecodable for UnsignedNodeAnnouncement {
 					read_pos += 1 + 12;
 				},
 				4 => {
-					if v.len() < read_pos + 1 + 37 {
-						return Err(DecodeError::ShortRead);
-					}
 					if addresses.len() > 3 || (addresses.len() > 0 && addresses.last().unwrap().get_id() > 3) {
 						return Err(DecodeError::ExtraAddressesPerType);
 					}
+					if addr_read_limit < read_pos + 1 + 37 {
+						return Err(DecodeError::BadLengthDescriptor);
+					}
 					let mut ed25519_pubkey = [0; 32];
 					ed25519_pubkey.copy_from_slice(&v[read_pos + 1..read_pos + 33]);
 					addresses.push(NetAddress::OnionV3 {