Fix bug where is_canonical() would erroneously return true

dwrensha · dwrensha · commit e72746cdd4c6 · 2018-03-08T19:09:19.000-05:00
on messages with certain pathological intersegment pointers.

The bug was reported by Mathias Svensson of Seasoned Software.
diff --git 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<bool> {
+        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
@@ -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());
+}
