capnproto / capnproto-java

Cap'n Proto in pure Java
Other
391 stars 86 forks source link

Fix SegmentReader bugs #134

Closed jonahbeckford closed 1 year ago

jonahbeckford commented 1 year ago

There are two bugs, one for each of the two first commits:

  1. Support signed offsets for Structs and Lists pointers - without this change perfectly valid segments are rejected
  2. Word arithmetic should be used for bounds checking - without this change we have a safety violation

1 - Support signed offsets for Structs and Lists pointers

Both Structs and Lists support two's complement "Signed" offsets:

B (30 bits) = Offset, in words, from the end of the pointer to the start of the struct's data section. Signed.

B (30 bits) = Offset, in words, from the end of the pointer to the start of the first element of the list. Signed.

The prior code only supported positive offsets because it used the Java >>> operator, which is an unsigned shift operator. The Java >> operator is the signed shift operator.

Audited the remaining uses of the shift operator; they were correct and they are documented as such.

Positive offsets are only guaranteed in a Canonical message.

2 - Word arithmetic should be used for bounds checking

Prior to this change, the start index was treated as a byte index rather than a word index.

dwrensha commented 1 year ago

Wow! Good catch!