Skip to content

fix(appsec): off by one#3506

Merged
realFlowControl merged 1 commit intomasterfrom
florian/fix-off-by-one
Nov 28, 2025
Merged

fix(appsec): off by one#3506
realFlowControl merged 1 commit intomasterfrom
florian/fix-off-by-one

Conversation

@realFlowControl
Copy link
Copy Markdown
Member

@realFlowControl realFlowControl commented Nov 28, 2025

Description

sizeof("1") returns 2 because it includes the terminating \0, while sprintf() always also writes the terminating \0. So if we do this (decrementing the return of sizeof("string"):

char buf[sizeof("18446744073709551615") - 1];
size_t len = sprintf(buf, "%zu", body_len);

... then there is an off by one, as the buf is "only" 20 bytes, while sprintf() might actually write 21 bytes to that pointer if the number in body_len needs 20 digits in a string. Additionally switching to snprintf() makes sure we never overflow.

Granted: this number overflowing seems highly unlikely, but theoretically still possible.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.69%. Comparing base (0c35caf) to head (3fa72d2).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3506      +/-   ##
==========================================
- Coverage   61.83%   61.69%   -0.14%     
==========================================
  Files         142      142              
  Lines       12923    12923              
  Branches     1695     1695              
==========================================
- Hits         7991     7973      -18     
- Misses       4178     4196      +18     
  Partials      754      754              
Files with missing lines Coverage Δ
appsec/src/extension/request_abort.c 73.86% <100.00%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c35caf...3fa72d2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@realFlowControl realFlowControl marked this pull request as ready for review November 28, 2025 11:17
@realFlowControl realFlowControl marked this pull request as ready for review November 28, 2025 11:17
@realFlowControl realFlowControl requested review from a team as code owners November 28, 2025 11:17
@realFlowControl realFlowControl merged commit c4ac6c5 into master Nov 28, 2025
1882 of 2008 checks passed
@realFlowControl realFlowControl deleted the florian/fix-off-by-one branch November 28, 2025 11:18
@github-actions github-actions Bot added this to the 1.15.0 milestone Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants