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/bytemath: Test multiplier operator #1244

Closed
wants to merge 1 commit into from

Conversation

jlucovsky
Copy link
Contributor

Issue: 6070

This commit adds a test for the byte-math multiplication operator. The operator was missing from 6.0.x; however, this test applies to 6.0.x and later once the Suricata PR is merged.

@catenacyber catenacyber added the requires suricata pr Depends on a PR in Suricata label Jun 12, 2023
@catenacyber
Copy link
Collaborator

Requires OISF/suricata#9010

@victorjulien victorjulien added the tests pass These new tests should pass label Jun 28, 2023
@catenacyber catenacyber removed the requires suricata pr Depends on a PR in Suricata label Jun 29, 2023
Copy link
Collaborator

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

@jlucovsky could you address the comments ?

Most important is to test for some exact value (cf problems we had with smb negotiate flags where the test was green because wa gad one chance out of 2 to have the right boolean value by picking it at an arbitrary offset)

Copy link
Member

@jasonish jasonish left a comment

Choose a reason for hiding this comment

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

Comments inline.

@jlucovsky
Copy link
Contributor Author

@jlucovsky could you address the comments ?

Most important is to test for some exact value (cf problems we had with smb negotiate flags where the test was green because wa gad one chance out of 2 to have the right boolean value by picking it at an arbitrary offset)

Good point ... I've modified the rule to look for an exact match.

Issue: 6070

This commit adds a test for the byte-math multiplication operator. The
operator was missing from 6.0.x; however, this test applies to 6.0.x and
later once the Suricata PR is merged.
Copy link
Collaborator

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

Thanks for the changes

@jasonish jasonish mentioned this pull request Sep 19, 2023
@jasonish jasonish mentioned this pull request Sep 28, 2023
@victorjulien
Copy link
Member

Merged in #1244, thanks!

@jlucovsky jlucovsky deleted the 6070/1 branch September 29, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests pass These new tests should pass
4 participants