Skip to content

stream: remove incorrect defensive check#13763

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

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

Conversation

@inashivb

Copy link
Copy Markdown
Member

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7636

Previous PR: #13762

Changes since v1:

  • corrected commit message

@inashivb inashivb requested a review from victorjulien as a code owner August 28, 2025 10:17
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 or if packet 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 force-pushed the tcp-closing-assertion/v2 branch from ddbc00d to a1b64df Compare August 28, 2025 10:18
@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 (a1b64df).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13763      +/-   ##
==========================================
- Coverage   83.72%   83.71%   -0.02%     
==========================================
  Files        1011     1011              
  Lines      275068   275067       -1     
==========================================
- Hits       230309   230280      -29     
- Misses      44759    44787      +28     
Flag Coverage Δ
fuzzcorpus 62.93% <ø> (-0.01%) ⬇️
livemode 19.00% <ø> (-0.02%) ⬇️
pcap 44.72% <ø> (-0.02%) ⬇️
suricata-verify 65.08% <ø> (+<0.01%) ⬆️
unittests 59.18% <ø> (+<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.

@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.

LGTM :-)

@victorjulien victorjulien added this to the 8.0 milestone Aug 28, 2025
@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 27268

@victorjulien

Copy link
Copy Markdown
Member

Merged in #13767, thanks!

@inashivb inashivb deleted the tcp-closing-assertion/v2 branch October 7, 2025 09:53
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.

4 participants