Skip to content

next/1469/20260828/v1 - #16141

Merged
victorjulien merged 7 commits into
OISF:mainfrom
victorjulien:next/1469/20260828/v1
Aug 29, 2026
Merged

next/1469/20260828/v1#16141
victorjulien merged 7 commits into
OISF:mainfrom
victorjulien:next/1469/20260828/v1

Conversation

jlucovsky and others added 7 commits August 28, 2026 14:49
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
Fail rule load when byte_math pairs << or >> with a literal rvalue of
64 or more. rvalue was bounded only to u32::MAX, so a rule shifting by
100 loaded and then produced 0 for every packet it inspected, spending
detection work on a comparison whose outcome was settled before the
first packet arrived.

A variable rvalue still loads. Its value comes from a byte_extract on
the packet and is not known until the rule runs, so the guard in
DetectByteMathDoMatch() stays the only check covering that path.

DetectByteMathParse() reports every SCByteMathParse() failure as
"invalid bytemath values", so the rejected rule is named by the
"error parsing signature" line that follows rather than by the reason
the parser gave.

Issue: 8845
Record that << and >> yield 0 when rvalue is 64 or more, beside the
existing note about division by zero. A shift count can come from a
byte_extract variable, so the rule text alone does not say which
counts land in that case.

Issue: 8845
In very large rulesets with lots of different patterns the
size calculation could overflow.

Ticket: OISF#8844.
When reading an ERF file do not trust the ERF header values.

Do not use wlen to set packet length.

Ticket: 8836

Read rlen bytes into MAX_PAYLOAD_SIZE buffer.
Use PacketCopyData() to safely update p.

Ticket: 8865
Add support for ERF record types META and PAD, plus additional ETH types.

Add support for ERF extension headers.

These types and extension headers were already supported by erf-source-dag.

Made source-erf-file and source-erf-dag more consistent.

Ticket: 8962
When multithreaded output was enabled, all threads freed
shallow-copy of sensor and prefix names.

This commit adds a guard so that only the parent frees the
allocated buffers.

Ticket: 8861
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.03030% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.09%. Comparing base (389700e) to head (7ca9471).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16141      +/-   ##
==========================================
+ Coverage   83.05%   83.09%   +0.03%     
==========================================
  Files        1004     1004              
  Lines      277407   277514     +107     
==========================================
+ Hits       230410   230603     +193     
+ Misses      46997    46911      -86     
Flag Coverage Δ
fuzzcorpus 61.48% <18.55%> (-0.02%) ⬇️
livemode 18.41% <8.24%> (-0.01%) ⬇️
netns 22.86% <8.24%> (-0.05%) ⬇️
pcap 45.39% <8.24%> (+<0.01%) ⬆️
suricata-verify 67.30% <26.80%> (+0.05%) ⬆️
unittests 58.53% <48.48%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa

Copy link
Copy Markdown

Information:

ERROR: QA failed on IPS_AFP_drop_chk.

field baseline test %
IPS_AFP_stats_chk
.ips.blocked 706320 758806 107.43%
.ips.drop_reason.exception_policy_flow_drop 0 48498 -
.ips.drop_reason.applayer_error 0 431 -
.ips.drop_reason.stream_error 0 3716 -
.ips.drop_reason.stream_midstream 0 58 -
.capture.kernel_drops 0 37212 -
.tcp.reassembly_gap 54000 56042 103.78%
.tcp.overlap 0 3 -
.flow.end.state.new 5400 5636 104.37%
.flow.end.tcp_state.syn_sent 0 111 -
.flow.end.tcp_state.fin_wait1 0 22 -
.flow.end.tcp_state.fin_wait2 0 15 -
.flow.end.tcp_state.time_wait 0 8 -
.flow.end.tcp_state.last_ack 0 12 -
.flow.end.tcp_state.close_wait 0 25 -
.exception_policy.tcp.midstream.drop_flow 0 58 -
.exception_policy.app_layer.error.drop_flow 0 432 -
.app_layer.error.http.parser 0 6 -
.app_layer.error.ftp.gap 0 2 -
.app_layer.error.smtp.gap 0 13 -
.app_layer.error.tls.gap 0 398 -
.app_layer.error.ssh.gap 0 1 -
.app_layer.error.dcerpc_tcp.parser 0 11 -
.app_layer.error.pop3.parser 0 1 -

Pipeline = 33444

@jufajardini

Copy link
Copy Markdown
Contributor

Consistent with original, approved, PRs.
CI checks.
Are the baseline failures expected?

@jasonish jasonish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Staging looks OK.. I'm not sure about the QA test, doesn't look related?

@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 33455

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.

6 participants