Skip to content

fix: Import math and validate request body size - #7535

Open
odaysec wants to merge 2 commits into
elastic:mainfrom
odaysec:patch-1
Open

fix: Import math and validate request body size#7535
odaysec wants to merge 2 commits into
elastic:mainfrom
odaysec:patch-1

Conversation

@odaysec

@odaysec odaysec commented Aug 1, 2026

Copy link
Copy Markdown

Add math import and check for bulk request body size. fix protect all allocation-size arithmetic by checking for overflow before computing/using capacities. For this specific issue, make Buf.grow compute a target capacity without overflowing, and panic with a clear message if growth cannot be represented as int.

Best targeted fix (no functional change beyond safety):

  • Edit internal/pkg/danger/buf.go, function grow.
  • Replace 2*cap(b.buf)+n with guarded arithmetic:
    • Validate n >= 0 (defensive, even though Grow checks).
    • Compute required capacity need := len(b.buf) + n with overflow check.
    • Compute doubled capacity from current cap with overflow check/saturation.
    • Choose newCap = max(need, doubledCap).
    • Allocate with make([]byte, len(b.buf), newCap).
  • Add math import for math.MaxInt.

Design Checklist

  • I have ensured my design is stateless and will work when multiple fleet-server instances are behind a load balancer.
  • I have or intend to scale test my changes, ensuring it will work reliably with 100K+ agents connected.
  • I have included fail safe mechanisms to limit the load on fleet-server: rate limiting, circuit breakers, caching, load shedding, etc.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool

Related issues

Add math import and check for bulk request body size. fix protect all allocation-size arithmetic by checking for overflow before computing/using capacities. For this specific issue, make `Buf.grow` compute a target capacity without overflowing, and panic with a clear message if growth cannot be represented as `int`.

Best targeted fix (no functional change beyond safety):
- Edit `internal/pkg/danger/buf.go`, function `grow`.
- Replace `2*cap(b.buf)+n` with guarded arithmetic:
  - Validate `n >= 0` (defensive, even though `Grow` checks).
  - Compute required capacity `need := len(b.buf) + n` with overflow check.
  - Compute doubled capacity from current cap with overflow check/saturation.
  - Choose `newCap = max(need, doubledCap)`.
  - Allocate with `make([]byte, len(b.buf), newCap)`.
- Add `math` import for `math.MaxInt`.
@odaysec
odaysec requested a review from a team as a code owner August 1, 2026 22:37
@odaysec
odaysec requested review from blakerouse and lorienhu August 1, 2026 22:37
@mergify

mergify Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

This pull request does not have a backport label. Could you fix it @ekuboo100? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

Comment thread internal/pkg/danger/buf.go

// Serialize request
const kSlop = 64
if len(body) > math.MaxInt-kSlop {

@kruskall kruskall Aug 2, 2026

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.

Could you share what system you are running to have a byte slice of len(math.MaxInt) in memory ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants