backport 8845 into main-8.0.x- #16142 - #16143
Closed
jlucovsky wants to merge 3 commits into
Closed
Conversation
Zero the result when a byte_math right shift count reaches 64, the width of the uint64_t being shifted, so the operation no longer depends on behavior C11 6.5.7p3 leaves undefined. The left shift case has done this since 473ca6d; the right shift case was left unguarded. DetectByteMathDoMatch() shifted by whatever count it was handed. On x86_64 the hardware masks the count to its low six bits, so a count of 64 became a shift of 0 and returned the extracted value unchanged instead of 0. That value is stored in det_ctx->byte_values[] and feeds any byte_test, isdataat, or content offset later in the signature, so the signature's verdict follows from an arithmetic result the standard does not define. The count reaches the shift from the wire. When byte_math names a variable for rvalue, DetectEngineContentInspectionInternal() reads it out of det_ctx->byte_values[] at detect-engine-content-inspection.c:614, where a preceding byte_extract stored bytes taken from the payload, so one payload byte of 0x40 sets the count to 64. Issue: 8845 Issue: 8902 (cherry picked from commit e5d035f)
Warn from DetectByteMathSetup() when byte_math pairs << or >> with a literal rvalue of 64 or more, naming the signature by sid. Such a shift gives 0 for every packet: DetectByteMathDoMatch() zeroes the result once the count reaches 64, the width of the uint64_t being shifted. rvalue was bounded only to the u32 range, so the rule loaded and ran a shift whose result was 0 whatever the packet held. The rule still loads. main rejects it in 89d09c4, the commit this one is derived from, but a released branch should not stop loading rules that loaded before. Passing byte_math to --strict-rule-keywords turns the warning into a load failure, the same escalation classtype and reference apply to values they cannot check. A variable rvalue draws no warning. It resolves to a preceding byte_extract or byte_math result, whose value is known only once the rule runs, so the guard in DetectByteMathDoMatch() stays the only check on that path. Issue: 8845 Issue: 8902 (cherry picked from commit 89d09c4)
Record beside the existing note about division by zero that << and >> give 0 when rvalue is 64 or more, that a rule giving such an rvalue as a number still loads with a warning, and that --strict-rule-keywords makes it fail to load instead. rvalue can name a byte_extract or byte_math variable rather than a number, and then the count is not known until the rule runs, so the rule text does not say whether it will reach 64. The note on main stops at the result, because 89d09c4 rejects such a rule there instead of warning about it. Issue: 8845 Issue: 8902 (cherry picked from commit 4a64778)
jlucovsky
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
August 30, 2026 12:56
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main-8.0.x #16143 +/- ##
==============================================
- Coverage 81.73% 81.72% -0.01%
==============================================
Files 1017 1017
Lines 277278 277339 +61
==============================================
+ Hits 226628 226655 +27
- Misses 50650 50684 +34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 33460 |
victorjulien
approved these changes
Aug 31, 2026
Member
|
Merged in #16146, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuation of #16142
Cherry-pick 8845 fixes into main-8.0.x
Link to ticket: https://redmine.openinfosecfoundation.org/issues/8902
Describe changes:
--strict-rule-keywordsis used.Updates:
Provide values to any of the below to override the defaults.
link to the pull request in the respective
_BRANCHvariable.SV_REPO=
SV_BRANCH=OISF/suricata-verify#3336
SU_REPO=
SU_BRANCH=