Skip to content

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

Open
jdomeracki-coder wants to merge 1 commit into
release/2.33from
cherry-pick/25710/release/2.33
Open

fix: validate FileSize in NewDataBuilder to prevent OOM DoS (#25710)#26185
jdomeracki-coder wants to merge 1 commit into
release/2.33from
cherry-pick/25710/release/2.33

Conversation

@jdomeracki-coder

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

Copy link
Copy Markdown
Contributor

Backport of #25710 to release/2.33.

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.33.

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.

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