&max-size will overread and only raise error afterwards

[0xxon]

The &max-size attribute will happily allow inner units to over-read the data from the input stream till they are done, including raising hooks (or zeek events) - only raising an error after all parsing is finished.

Example:

module Test;

public type Top = unit {
  innerdata: Inner &max-size=2;
};

type Inner = unit {
   data: bytes &size=20;
   on %done { print self; }
};

$ printf '\x0b\x00\x00\x01\x00\x0e\xec\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' | spicy-driver test2.spicy
[$data=b"\x0b\x00\x00\x01\x00\x0e\xec\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"]
[error] terminating with uncaught exception of type spicy::rt::ParseError: parsing not done within &max-size bytes (test2.spicy:4:20-4:30)

This obviously is a problem, especially in cases when one parses a protocol that specifies unit-lengths itself.

To give a real-world example: This bug was found while examining a test-failure fo the TLS testsuite. In that case, an inner field specifies a much-too-large-length, exceeding the size of the outer fields by several kilobytes. As this overread seems to exceed the amount of data remaining in this connection, no error is ever raised.

[bbannier]

What happens here is that we set up two dependent limited views into the input stream:

1. a View view1 with size=3 to parse innerdata from (one larger than the &max-size value to accommodate a sentinel byte for detecting overruns)
2. a View view2 from view1 with size=20 to parse data from

This fails since when calling limit on an already limited view but with a larger size, the original limit is overriden. This is an issue in the implementation of hilti::rt::stream::View::limit, and can reproduced with this unit test:

SUBCASE("limited view inherits limit") {
    auto input = "1234567890"_b;
    auto stream = Stream(input);
    auto view = stream.view();

    auto limit1 = view.limit(5U);
    REQUIRE_EQ(limit1.size(), 5U);

    auto limit2 = limit1.limit(input.size());
    REQUIRE_EQ(limit2.size(), 5U);
}

The fix would probably be to only ever decrease the size of a View with limit. The only downside of that would be that parsing would then fail since data cannot read enough data, and we do not explicitly trigger an error pointing out&max-size anymore. With that approach we also seem to fail a number of tests, so we might have assumptions about limit being able to override elsewhere.