zeek / spicy

C++ parser generator for dissecting protocols & files.
https://docs.zeek.org/projects/spicy
Other
246 stars 35 forks source link

Bytes vector compiles without termination condition for bytes field. #1832

Closed sethhall closed 1 week ago

sethhall commented 1 month ago

This toy parser is applying size to the total size of the vector but the bytes in the vector actually have no termination condition.

module test;

public type X = unit {
    a: bytes[10] &size=5;
};
rsmmr commented 1 month ago

Looks like there's validation missing rejecting the inner field: it should use the same logic that we use for bytes fields outside of vectors to enforce that there's some termination condition (e.g, &eod, &size, etc.).

evantypanski commented 1 week ago

Well it seems like the validator is trying, but it passes the vector's field through during validation: https://github.com/zeek/spicy/blob/64c0000c2a9385020e7f357711c0da3de4b03517/spicy/toolchain/src/compiler/validator.cc#L147-L152

If you surround bytes in parentheses, this actually gets you the error properly:

module test;

public type X = unit {
    a: (bytes)[10] &size=5;
};

The only change is parentheses around bytes. That gets me:

[error] test.spicy:4:9-4:13: bytes field requires one of &eod, &parse_at, &parse_from, &size, &until, &until-including
[error] spicyc: aborting after errors

I'm not quite sure how to get the attributes down to the inner bytes type since the attributes seem applied to fields not types, but I'll look into that :)

bbannier commented 1 week ago

Fixed with f9bbf82b7cadbf31f0e606708c380f42022847cc.