From 5ab33a97b55d780e9a2dde996abcffcaa216948d Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Thu, 13 Aug 2020 22:17:59 -0600 Subject: [PATCH 1/3] Add failing test for decoding ShortU16 alias values --- sdk/src/short_vec.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sdk/src/short_vec.rs b/sdk/src/short_vec.rs index 3f9e94e3dc769c..8eb3006a35a1c0 100644 --- a/sdk/src/short_vec.rs +++ b/sdk/src/short_vec.rs @@ -246,4 +246,17 @@ mod tests { let s = serde_json::to_string(&vec).unwrap(); assert_eq!(s, "[[3],0,1,2]"); } + + #[test] + fn test_decode_len_aliased_values() { + let one1 = [0x01]; + let one2 = [0x81, 0x00]; + let one3 = [0x81, 0x80, 0x00]; + let one4 = [0x81, 0x80, 0x80, 0x00]; + + assert_eq!(decode_len(&one1).unwrap(), (1, 1)); + assert_eq!(decode_len(&one2).unwrap(), (1, 2)); + assert_eq!(decode_len(&one3).unwrap(), (1, 3)); + assert!(decode_len(&one4).is_err()); + } } From ec3f6d017f0b531a4adbc7364eda094db1a949c9 Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Thu, 13 Aug 2020 22:19:01 -0600 Subject: [PATCH 2/3] Factor out ShortU16 deser vistor logic to helper --- sdk/src/short_vec.rs | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/sdk/src/short_vec.rs b/sdk/src/short_vec.rs index 8eb3006a35a1c0..f8368b9b1c2580 100644 --- a/sdk/src/short_vec.rs +++ b/sdk/src/short_vec.rs @@ -38,6 +38,26 @@ impl Serialize for ShortU16 { } } +enum VisitResult { + Done(usize, usize), + More(usize, usize), + Err, +} + +fn visit_byte(elem: u8, len: usize, size: usize) -> VisitResult { + let len = len | (elem as usize & 0x7f) << (size * 7); + let size = size + 1; + let more = elem as usize & 0x80 == 0x80; + + if size > size_of::() + 1 { + VisitResult::Err + } else if more { + VisitResult::More(len, size) + } else { + VisitResult::Done(len, size) + } +} + struct ShortLenVisitor; impl<'de> Visitor<'de> for ShortLenVisitor { @@ -58,15 +78,16 @@ impl<'de> Visitor<'de> for ShortLenVisitor { .next_element()? .ok_or_else(|| de::Error::invalid_length(size, &self))?; - len |= (elem as usize & 0x7f) << (size * 7); - size += 1; - - if elem as usize & 0x80 == 0 { - break; - } - - if size > size_of::() + 1 { - return Err(de::Error::invalid_length(size, &self)); + match visit_byte(elem, len, size) { + VisitResult::Done(l, _) => { + len = l; + break; + } + VisitResult::More(l, s) => { + len = l; + size = s; + } + VisitResult::Err => return Err(de::Error::invalid_length(size + 1, &self)), } } From cbd875e60287e392609e60fd58720544aa605770 Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Thu, 13 Aug 2020 22:20:22 -0600 Subject: [PATCH 3/3] Reimplement decode_len() with ShortU16 vistor helper --- perf/src/sigverify.rs | 7 ++++--- sdk/src/short_vec.rs | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/perf/src/sigverify.rs b/perf/src/sigverify.rs index 9b797d3bde5829..44f0cf226b73df 100644 --- a/perf/src/sigverify.rs +++ b/perf/src/sigverify.rs @@ -131,7 +131,8 @@ fn do_get_packet_offsets( } // read the length of Transaction.signatures (serialized with short_vec) - let (sig_len_untrusted, sig_size) = decode_len(&packet.data)?; + let (sig_len_untrusted, sig_size) = + decode_len(&packet.data).map_err(|_| PacketError::InvalidShortVec)?; // Using msg_start_offset which is based on sig_len_untrusted introduces uncertainty. // Ultimately, the actual sigverify will determine the uncertainty. @@ -156,8 +157,8 @@ fn do_get_packet_offsets( } // read the length of Message.account_keys (serialized with short_vec) - let (pubkey_len, pubkey_len_size) = - decode_len(&packet.data[message_account_keys_len_offset..])?; + let (pubkey_len, pubkey_len_size) = decode_len(&packet.data[message_account_keys_len_offset..]) + .map_err(|_| PacketError::InvalidShortVec)?; if (message_account_keys_len_offset + pubkey_len * size_of::() + pubkey_len_size) > packet.meta.size diff --git a/sdk/src/short_vec.rs b/sdk/src/short_vec.rs index f8368b9b1c2580..4e0c59dee2e6d7 100644 --- a/sdk/src/short_vec.rs +++ b/sdk/src/short_vec.rs @@ -199,10 +199,20 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for ShortVec { } /// Return the decoded value and how many bytes it consumed. -pub fn decode_len(bytes: &[u8]) -> Result<(usize, usize), Box> { - let short_len: ShortU16 = bincode::deserialize(bytes)?; - let num_bytes = bincode::serialized_size(&short_len)?; - Ok((short_len.0 as usize, num_bytes as usize)) +pub fn decode_len(bytes: &[u8]) -> Result<(usize, usize), ()> { + let mut len = 0; + let mut size = 0; + for byte in bytes.iter() { + match visit_byte(*byte, len, size) { + VisitResult::More(l, s) => { + len = l; + size = s; + } + VisitResult::Done(len, size) => return Ok((len, size)), + VisitResult::Err => return Err(()), + } + } + Err(()) } #[cfg(test)]