Skip to content

stream: remove incorrect defensive check#13762

Closed
inashivb wants to merge 1 commit into
OISF:masterfrom
inashivb:tcp-closing-assertion/v1
Closed

stream: remove incorrect defensive check#13762
inashivb wants to merge 1 commit into
OISF:masterfrom
inashivb:tcp-closing-assertion/v1

Conversation

@inashivb

Copy link
Copy Markdown
Member

As a part of the commit d096b98 a defensive check was added stating that
the stream must have EOF flag set if it is in TCP_CLOSING state or
above. However, this led to a false positive reported by oss-fuzz whose
analysis showed that this does not hold true for TCP_CLOSING state. It
does hold true only for TCP_CLOSED and if pkt has PKT_PSEUDO_STREAM_END
set.
TCP_CLOSING state correspond to an established flow hence the correct
course of action is to remove the assertion.

Bug 7636

Co-authored-by: Philippe Antoine <pantoine@oisf.net>
@inashivb inashivb requested a review from victorjulien as a code owner August 28, 2025 02:41
@codecov

codecov Bot commented Aug 28, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.71%. Comparing base (2a17ab6) to head (ab431a0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13762      +/-   ##
==========================================
- Coverage   83.72%   83.71%   -0.02%     
==========================================
  Files        1011     1011              
  Lines      275068   275067       -1     
==========================================
- Hits       230309   230274      -35     
- Misses      44759    44793      +34     
Flag Coverage Δ
fuzzcorpus 62.93% <ø> (-0.01%) ⬇️
livemode 19.00% <ø> (-0.01%) ⬇️
pcap 44.70% <ø> (-0.03%) ⬇️
suricata-verify 65.09% <ø> (+0.02%) ⬆️
unittests 59.17% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 27260

@catenacyber catenacyber left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

commit message :
and if pkt has PKT_PSEUDO_STREAM_END should be or if packet

@inashivb

Copy link
Copy Markdown
Member Author

Looks good to me

commit message : and if pkt has PKT_PSEUDO_STREAM_END should be or if packet

Very right. Please check v2

@inashivb inashivb closed this Aug 28, 2025
@inashivb inashivb deleted the tcp-closing-assertion/v1 branch August 28, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants