zeek / spicy

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

Segfault with array-switch-size-array-try construction #1795

Closed awelzel closed 1 month ago

awelzel commented 1 month ago

I've worked some more on the LDAP and managed to trigger a segfault related to chunk handling.

This is the minimal reproducer (using latest Zeek master, but also crashes within the zeek/zeek:6.2.1 container image.

$ echo CJ2RXS7YIm4Lb+s5Y3nb0v2MVYgrmvRzZPBPQNTS4zE= | base64 -d | spicy-driver --increment 11 ./x.spicy
Segmentation fault (core dumped)
module Test;

type Inner = unit {
  len: uint8;
  magic: bytes &size=self.len % 3;
};

type TryInner = unit {
  : Inner &try { }
};

type Message = unit {
  len: uint8;

  switch ( True ) {
    True -> :TryInner[] &eod;
    * -> skip bytes &eod;
  } &size=self.len;
};

public type Messages = unit {
  : Message[];
};

Wasn't planning on using such a construct, but the extensions to the existing LDAP parser ended like so. I haven't tried to further reduce it.

Test data was produced as follows:

c=0; while [ $c -eq 0 ]; do dd if=/dev/urandom of=test.data count=1 bs=32; spicy-driver -f test.data  --increment 11 ./x.spicy ; if [ $? -gt 100 ] ; then echo "SIGNAL"; break; fi; done

Backtrace

#0  hilti::rt::stream::detail::Chunk::offset (this=0x0) at /home/awelzel/corelight-oss/zeek/auxil/spicy/hilti/runtime/include/hilti/rt/types/stream.h:185
#1  hilti::rt::stream::View::extract (this=this@entry=0x7ffff6ffca40, dst=dst@entry=0x7ffff6ffc977 "", n=n@entry=1) at /home/awelzel/corelight-oss/zeek/auxil/spicy/hilti/runtime/include/hilti/rt/types/stream.h:1471
#2  0x00007ffff7e6cfd5 in hilti::rt::integer::unpack<unsigned char, hilti::rt::stream::View> (b=..., fmt=fmt@entry=...) at /home/awelzel/corelight-oss/zeek/auxil/spicy/hilti/runtime/include/hilti/rt/types/integer.h:161
#3  0x00007ffff7e6257c in __hlt::Test::Message::__parse_Test__Message_stage2 (this=this@entry=0x5555556d7fa0, __data=..., __begin=..., __cur=..., __trim=__trim@entry=..., __lah=..., __lahe=..., __error=std::optional<hilti::rt::RecoverableFailure> [no contained value])
    at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:287
#4  0x00007ffff7e6578b in __hlt::Test::Message::__parse_stage1 (this=this@entry=0x5555556d7fa0, __data=..., __begin=..., __cur=..., __trim=__trim@entry=..., __lah=..., __lahe=..., __error=std::optional<hilti::rt::RecoverableFailure> [no contained value]) at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:366
#5  0x00007ffff7e674a9 in __hlt::Test::Messages::__parse__anon_3_stage1 (this=this@entry=0x5555555ee680, __data=..., __begin=..., __cur=..., __trim=..., __lah=..., __lahe=..., __error=std::optional<hilti::rt::RecoverableFailure> [no contained value], __dst=...) at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:520
#6  0x00007ffff7e669d5 in __hlt::Test::Messages::__parse_Test__Messages_stage2 (this=this@entry=0x5555555ee680, __data=..., __begin=..., __cur=..., __trim=..., __trim@entry=..., __lah=..., __lahe=..., __error=std::optional<hilti::rt::RecoverableFailure> [no contained value])
    at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:478
#7  0x00007ffff7e6806b in __hlt::Test::Messages::__parse_stage1 (this=0x5555555ee680, __data=..., __begin=..., __cur=..., __trim=__trim@entry=..., __lah=..., __lahe=..., __error=std::optional<hilti::rt::RecoverableFailure> [no contained value]) at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:556
#8  0x00007ffff7e6952f in __hlt::Test::Messages::parse3 (__gunit=..., __data=..., __cur=std::optional<hilti::rt::stream::View> [no contained value], __context=...) at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:629
#9  0x00007ffff7e6cb8d in hlt::Test::Messages::parse3(hilti::rt::ValueReference<spicy::rt::ParsedUnit>&, hilti::rt::ValueReference<hilti::rt::Stream>&, std::optional<hilti::rt::stream::View> const&, std::optional<spicy::rt::UnitContext> const&)::$_2::operator()(hilti::rt::detail::Fiber*) const (
    this=<optimized out>, r=<optimized out>) at /tmp/Test_b3b8f96d093109dc-38d59553b7c4c58e.cc:772
bbannier commented 1 month ago

Interestingly that issue seems to go away if I make the following replacement

-     * -> skip bytes &eod;
+     * -> : skip bytes &eod;

I am not even sure what code we generate for the original default case.

awelzel commented 1 month ago

Interestingly that issue seems to go away if I make the following replacement

Hmm, not here. Also continues to crash when using xs: bytes &eod instead (the --increment 11 is important, maybe you left that off?)

bbannier commented 1 month ago

Hmm, not here. Also continues to crash when using xs: bytes &eod instead (the --increment 11 is important, maybe you left that off?)

You are right, I must have messed something up.

The issue here seems to be between parsing some sized bytes which are bigger than an outside &size constraint. A reduced reproducer seems to be

module Test;

type Message = unit {
    : uint8;
    : (bytes &size=2) &size=1; # Same behavior if parsing list of parser in (..).
};

public type Messages = unit {
    # Need to parse at least two to break.
    : Message;
    : Message;
};

The actual failure is covered by a failing assertion which is only checked in debug mode.

$ spicy-driver test.spicy -d -f /dev/zero
Assertion failed: (chain->inRange(p.offset())), function extract, file stream.h, line 1464.
[5]    2982 abort      spicy-driver test.spicy -d -f /dev/zero

Marking as regression since this regressed with #1647 which was also backported to v1.10.0.