Skip to content

Explicitly Handle Optionals Tags of the Form 0bxxx11111 #5259

@InsertCreativityHere

Description

@InsertCreativityHere

The current structure for optional tags in Ice is as follows:

  • The first 3 bits represent the type of the tag (F4, VSize, etc.)
  • The last 5 bits represent the value of the tag. With 5 bits, this value must be in the range [0, 31]
  • If the tag value doesn't fit in this range, we encode a value of 30, followed by the actual value encoded as a size.

There are 2 special cases that our decoding handles:

  • If the full byte is 255 (0b11111111): this is the OPTIONAL_END_MARKER, which means there's no more tags.
  • Like we already mentioned, if the value is 30 (0bxxx11110), this means the tag value follows as a size.

But this raises the natural question. If the range is [0, 31], and 30 is a special value... what about 31?
I guess we avoided 31 to avoid confusion with OPTIONAL_END_MARKER. But this is actually not possible now that we disallow optional classes.


Regardless, if the other side sends a tag byte of the form 0bxxx11111 (other than 0b11111111), our decoding will fail in unpredictable ways, usually a MarshalException, but not always. And there's at least 1 way to achieve a silent failure state.

If the other side sends a tag of 0b10011111, we only decode 1 byte (since it doesn't end with 30 (0b11110)).
If the decoder is looking for a tag value smaller than 30 (likely), it sees this tag is too large, and will then rewind.
Instead of rewinding by the 1 byte it read though, it rewinds by 2 bytes (since it always assumes 31 is written on 2 bytes).
This means the decoder is positioned 1 byte earlier than it should be, possibly outside the memory it should be reading from.

It also just so happens that we tolerate an extra byte in the encapsulation. So, this off-by-one shift could go entirely unnoticed:

if (_buf.b.position() + 1 != _encapsStack.start + _encapsStack.sz) {
throw new MarshalException("Failed to unmarshal encapsulation.");
}
// Ice version < 3.3 had a bug where user exceptions with class members could be encoded with a
// trailing byte when dispatched with AMD. So we tolerate an extra byte in the encapsulation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions