Skip to content

smtp: check transaction before to-client completion - v1 - #15869

Open
jasonish wants to merge 1 commit into
OISF:mainfrom
jasonish:smtp/ossfuzz/v1
Open

smtp: check transaction before to-client completion - v1#15869
jasonish wants to merge 1 commit into
OISF:mainfrom
jasonish:smtp/ossfuzz/v1

Conversation

@jasonish

Copy link
Copy Markdown
Member

Ticket: https://redmine.openinfosecfoundation.org/issues/8739

SV_BRANCH=OISF/suricata-verify#3231

Make sure the transaction still exists before completing it in the
to-client direction. A pipelined RSET reply may already have completed and
freed it while a later end-of-DATA marker still refers to it.

Found by OSS-Fuzz testcase 5498180758994944.

Make sure the transaction still exists before completing it in the
to-client direction. A pipelined RSET reply may already have completed and
freed it while a later end-of-DATA marker still refers to it.

Found by OSS-Fuzz testcase 5498180758994944.

Bug OISF#8739.
@jasonish
jasonish requested a review from victorjulien as a code owner July 13, 2026 17:22
@jasonish

Copy link
Copy Markdown
Member Author

AI-generated review posted automatically by Suricata ai-review.

Verdict: approve

No actionable findings.

Redmine #8739: The null guard at src/app-layer-smtp.c:1084 prevents completion of a reclaimed transaction without falling back to an unrelated curr_tx, satisfying the ticket.

Linked suricata-verify PR #3231 provides adequate coverage. Its minimized 13-packet capture reproduces the pipelined RSET/DATA sequence; assertion-enabled main jobs fail pre-fix with exit -6, while the Suricata PR’s debug-validation job tests the fixed head using this fixture.

Validation: reviewed the complete one-commit diff from merge base 8455efd9, SMTP ownership/cleanup paths, PR timelines, Redmine #8739, and every linked test file and packet. git diff --check passed and the worktree remained clean. Per review instructions, no builds or tests were run locally. Broader CI, including the fixed-head debug-validation job, was still running.

Investigator and adjudicator both used Codex CLI with model gpt-5.6-sol at xhigh reasoning effort. Codex was the final adjudicator.

@suricata-qa

Copy link
Copy Markdown

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.flow.ftp_data 655 679 103.66%

Pipeline = 32569

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

Thanks for the work,

CI : ✅
Git ID set : looks fine for me
CLA : you already contributed :-p
Doc update : not needed
Redmine ticket : ok, make sure to include this in your backport of #15837
Tests : cool
Dependencies added: none
Code : good
Commits segmentation : simple enough
Commit messages : nice

@victorjulien victorjulien added this to the 9.0 milestone Aug 1, 2026
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