Bug #7636: tcp: assertion triggered in StreamTcpReassembleAppLayer - Suricata - Open Information Security Foundation

Actions

Copy link

Bug #7636

closed

tcp: assertion triggered in StreamTcpReassembleAppLayer

Added by Philippe Antoine 6 months ago. Updated 7 days ago.

Status:

Closed

Priority:

Normal

Assignee:

Shivani Bhardwaj

Target version:

8.0.1

Affected Versions:

8.0.0

Effort:

Difficulty:

Label:

Description

Found by oss-fuzz:
https://issues.oss-fuzz.com/u/1/issues/407269999

fuzz_predefpcap_aware: stream-tcp-reassemble.c:1425: int StreamTcpReassembleAppLayer(ThreadVars *, TcpReassemblyThreadCtx *, TcpSession *, TcpStream *, Packet *, enum StreamUpdateDir): Assertion `!(((stream_flags & 2) == 0))' failed.

Regression range is https://github.com/OISF/suricata/compare/cd69955d7fca991d0ffc615f6ee7dce9dd20a3c5...834378ff887b3d6ac1903efb7a3e7164f593abd0

Reproducer is
./src/suricata -c fuzz.yaml -k none -r lol2.pcap --disable-detection -l log --runmode=single

but this does not trigger on my other device...

Files

lol2.pcap (283 Bytes) lol2.pcap

Philippe Antoine, 03/31/2025 01:51 PM

Actions

Copy link

Updated by Victor Julien 6 months ago

Target version changed from 8.0.0-beta1 to 8.0.0-rc1

Actions

Copy link

Updated by Philippe Antoine 4 months ago

Priority changed from Normal to High

Victor, how bad is this assertion ?

Actions

Copy link

Updated by Victor Julien 4 months ago

Target version changed from 8.0.0-rc1 to 9.0.0-beta1

Actions

Copy link

Updated by Philippe Antoine 4 months ago

I do not think this bug is bad.

I think it only happens in midstream with fin+syn packets after some first midstream packet

Impact would be some structures kept in memory a bit longer than needed...

Actions

Copy link

Updated by Philippe Antoine 3 months ago

Affected Versions 8.0.0 added

Actions

Copy link

Updated by Philippe Antoine about 1 month ago

@Shivani Bhardwaj you added the assertion in https://github.com/OISF/suricata/commit/d096b989c813dcc71154a9080cadaee927e45d24

It is not true if ssn->state TCP_CLOSING

What should we do ?
- remove the assertion
- use stream_flags | STREAM_EOF in AppLayerHandleTCPData
- have StreamGetAppLayerFlags consider ssn->state >= TCP_CLOSING. instead of ssn->state TCP_CLOSED

Actions

Copy link

Updated by Shivani Bhardwaj about 1 month ago

Philippe Antoine wrote in #note-6:

@Shivani Bhardwaj you added the assertion in https://github.com/OISF/suricata/commit/d096b989c813dcc71154a9080cadaee927e45d24

It is not true if ssn->state TCP_CLOSING

Thank you, Philippe! Indeed your analysis is correct.

What should we do ?
- remove the assertion
- use stream_flags | STREAM_EOF in AppLayerHandleTCPData
- have StreamGetAppLayerFlags consider ssn->state >= TCP_CLOSING. instead of ssn->state TCP_CLOSED

TCP_CLOSING seems to not be treated as EOF ref: in StreamTcpPacketSetState, flow's state is updated to "established" if TCP state was "closing" so perhaps removal of assertion is the best course of action. Lmk wdyt?

Actions

Copy link