Skip to content

dcerpc: mimic gap behavior for invalid data#14890

Closed
inashivb wants to merge 1 commit intoOISF:mainfrom
inashivb:dcerpc-invalid-data-handling/v7
Closed

dcerpc: mimic gap behavior for invalid data#14890
inashivb wants to merge 1 commit intoOISF:mainfrom
inashivb:dcerpc-invalid-data-handling/v7

Conversation

@inashivb
Copy link
Copy Markdown
Member

Previous PR: #14829

Changes since v6:

  • simplified condition check as per review
  • rebased on top of latest main

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

SV_BRANCH=OISF/suricata-verify#2904

Note: QA deviations on dcerpc stats are expected. The pcaps I got from QA lab showed no errors and an increased number of txs looking like the corresponding s-v test that was updated.

If invalid data is sent to the parser then instead of rejecting it at
the first few bytes that do not conform to the header standards, mimic
gap behavior and try to skip a few bytes until a possibly good DCERPC
record is found.

Ticket: 7251
@inashivb inashivb marked this pull request as ready for review February 24, 2026 10:26
@inashivb inashivb requested a review from jasonish as a code owner February 24, 2026 10:26
@suricata-qa
Copy link
Copy Markdown

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.error.dcerpc_tcp.parser 10319 3100 30.04%
.app_layer.tx.dcerpc_tcp 4180 6294 150.57%

Pipeline = 29852

@inashivb inashivb added the needs baseline update QA will need a new base line label Feb 28, 2026
@catenacyber
Copy link
Copy Markdown
Contributor

The SV test shows we lack support for multi-PDU, not that we need to "mimic gap behavior for invalid data"

More details :

  • packet 67 has end of PDU A and beginning of PDU B
  • packet 68 continues PDU B

Because we do not support multi PDU, we drop the first bytes of PDU B, and porcessing packet 68 like it were a fresh PDU, we fail

@catenacyber
Copy link
Copy Markdown
Contributor

@inashivb do you agree after our call that we should first implement multi-PDU support to fix the SV test ?

@catenacyber catenacyber added the needs rebase Needs rebase to main label Mar 5, 2026
Copy link
Copy Markdown
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

Needs rebase after the revert was merged

@catenacyber
Copy link
Copy Markdown
Contributor

I think this can be ckosed in favor of #15023

@inashivb
Copy link
Copy Markdown
Member Author

Closing as per comments by Philippe.

@inashivb inashivb closed this Mar 16, 2026
@inashivb inashivb deleted the dcerpc-invalid-data-handling/v7 branch March 16, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs baseline update QA will need a new base line needs rebase Needs rebase to main

Development

Successfully merging this pull request may close these issues.

3 participants