Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect/byte-jump: Permit variable name for nbytes value #9145

Closed
wants to merge 2 commits into from

Conversation

jlucovsky
Copy link
Contributor

Continuation of #9128

This PR adds the ability to use a variable name with the nbytes value in a byte_jump keyword option list.

Link to redmine ticket: 6105

Describe changes:

  • Add logic to detect and use variable names for nbytes in the byte_jump option list
  • Document variable name and add to Snort difference document.

Updates

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the pull request number.

SV_REPO=
SV_BRANCH=pr/1254
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Issue: 6105

This commit adds the ability for nbytes to be a variable when used with
the byte_jump keyword.
* safely cast it when extracting bytes
*/
if (data->flags & DETECT_BYTEJUMP_NBYTES_VAR) {
if (!DetectBytejumpValidateNbytes(data, nbytes)) {
Copy link
Member

Choose a reason for hiding this comment

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

If DetectBytejumpValidateNbytes can fail in this path, it will generate errors. If it can't, the check can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can refactor the validate function to only emit an error message when called as part of rule setup.

I think the validation should remain when a variable is used though.

Thoughts @victorjulien ?

Copy link
Member

Choose a reason for hiding this comment

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

Works for me. As long as we can't get in a path with per packet errors I'm good.

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.flow.memuse 592373632 516027536 87.11%
TREX_GENERIC_stats_chk
.capture.kernel_drops 0 1100 0.00

Pipeline 14991

@jlucovsky
Copy link
Contributor Author

Continued in #9157

@jlucovsky jlucovsky closed this Jul 6, 2023
@jlucovsky jlucovsky deleted the 6105/6 branch July 11, 2023 14:39
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