Skip to content

fix: validate FileSize in NewDataBuilder to prevent OOM DoS (#25710)#26190

Merged
f0ssel merged 1 commit into
release/2.32from
cherry-pick/25710/release/2.32
Jun 11, 2026
Merged

fix: validate FileSize in NewDataBuilder to prevent OOM DoS (#25710)#26190
f0ssel merged 1 commit into
release/2.32from
cherry-pick/25710/release/2.32

Conversation

@jdomeracki-coder

@jdomeracki-coder jdomeracki-coder commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Backport of #25710 to release/2.32.

Cherry-picked with git cherry-pick -x (a2e1ddb56f); the commit body references the original PR.

Generated by Coder Agents on behalf of @jdomeracki-coder.

`NewDataBuilder` allocated `make([]byte, 0, req.FileSize)` using the
client-supplied `int64` with no upper-bound check. The DRPC 4 MiB wire
cap limits message size but not the integer value, so a crafted message
with `FileSize = 1<<40` forces a 1 TiB allocation, triggering an
unrecoverable `runtime.throw` that kills the entire `coderd` process.

Add a `MaxFileSize` constant (100 MiB, matching `HTTPFileMaxBytes` in
`coderd/files.go`) and reject negative or oversized `FileSize`, plus
negative or excessive `Chunks`, before the allocation.
`BytesToDataUpload` also returns an error for oversized data to preserve
the encode/decode round-trip contract. Fix a pre-existing reversed
subtraction in the `Add()` overflow error message.

Closes https://linear.app/codercom/issue/PLAT-231

<details>
<summary>Implementation details</summary>

- `provisionersdk/proto/dataupload.go`: New exported `MaxFileSize`
constant; validation in `NewDataBuilder` and `BytesToDataUpload`. Fixed
reversed subtraction in `Add()` error.
- `provisionersdk/proto/dataupload_test.go`: New
`TestNewDataBuilderValidation` with 7 subtests.
- Updated all 5 callers of `BytesToDataUpload` for new error return.
- Audited all `make([]byte, ...)` in provisioner paths; no other
client-supplied sizes.

</details>

> Generated by Coder Agents on behalf of @f0ssel

(cherry picked from commit a2e1ddb)

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

Backport of #25710 to release/2.32.

Verified as a clean version backport: identical changed files and diff stats to the sibling backports of this fix on the other release branches, with no unrelated files touched. Approving as requested.

This review was generated by Coder Agents.

@f0ssel f0ssel merged commit 2cabbc3 into release/2.32 Jun 11, 2026
31 of 32 checks passed
@f0ssel f0ssel deleted the cherry-pick/25710/release/2.32 branch June 11, 2026 21:06
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants