From 66023b7886b821767b732a1354a5f486ff8da167 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 25 Jul 2018 15:27:19 -0400 Subject: [PATCH] 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. --- fuzz/fuzz_targets/channel_target.rs | 2 ++ src/ln/msgs.rs | 31 +++++++++++++++++------------ 2 files changed, 20 insertions(+), 13 deletions(-) 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; @@ -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 { -- 2.39.5