From e72746cdd4c672a4b8881ed2ed0375b69d1afb3a Mon Sep 17 00:00:00 2001 From: David Renshaw Date: Thu, 8 Mar 2018 18:56:09 -0500 Subject: [PATCH] Fix bug where `is_canonical()` would erroneously return `true` on messages with certain pathological intersegment pointers. The bug was reported by Mathias Svensson of Seasoned Software. --- capnp/src/private/layout.rs | 4 ++++ capnp/tests/canonicalize.rs | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/capnp/src/private/layout.rs b/capnp/src/private/layout.rs index eceeaa68e..16d19088d 100644 --- a/capnp/src/private/layout.rs +++ b/capnp/src/private/layout.rs @@ -2405,6 +2405,10 @@ impl <'a> PointerReader<'a> { } pub fn is_canonical(&self, read_head: &Cell<*const Word>) -> Result { + if self.pointer.is_null() || unsafe { !(*self.pointer).is_positional() } { + return Ok(false) + } + match try!(self.get_pointer_type()) { PointerType::Null => Ok(true), PointerType::Struct => { diff --git a/capnp/tests/canonicalize.rs b/capnp/tests/canonicalize.rs index ae9b35e8f..750c2ce08 100644 --- a/capnp/tests/canonicalize.rs +++ b/capnp/tests/canonicalize.rs @@ -512,3 +512,22 @@ fn out_of_bounds_zero_sized_void_list_returns_error() { let message = message::Reader::new(segment_array, Default::default()); assert!(message.is_canonical().is_err()); } + +#[test] +fn far_pointer_to_same_segment() { + let segment: &[Word] = &[ + // Far pointer to this same segment. Landing pad is two words, offset of one. + capnp_word!(0x0e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00), + + // Landing pad. Again, points back to this same segment. + capnp_word!(0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00), + + // Tag word, describing struct with 2-word data section. + capnp_word!(0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00), + ]; + + let segments = &[segment]; + let segment_array = message::SegmentArray::new(segments); + let message = message::Reader::new(segment_array, Default::default()); + assert!(!message.is_canonical().unwrap()); +}