fix(server): bound the pre-auth request-body read - #85
Open
0xKarm wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bound the pre-auth request-body read (
ReadRequestBody)Problem.
ReadRequestBodycallsio.ReadAll(r.Body)with no size limit, andthe charge middleware calls it on every request — before any credential is
checked — so it can compute/validate the body digest. Headers are already capped
at 16 KB (
pkg/mpp/parse.go), but the body is not. An unauthenticated clientcan POST arbitrarily large bodies to any protected route and force the server to
buffer each one in memory before it even returns the free
402. That's amemory-exhaustion DoS reachable without holding a payment credential. The core
net/httppath plus the Gin and Echo adapters all funnel throughReadRequestBodyand are affected; Fiber reads viac.Body()(fasthttp boundsthat itself) and is not.
Fix.
server.MaxRequestBodyBytes(default 1 MiB,0disables).ReadRequestBodynow reads throughio.LimitReader(r.Body, limit+1)andreturns an error when the body exceeds the limit, instead of buffering it all.
ErrBadRequest→ HTTP 400, so nocall-site changes are needed. The body is still fully restored for the handler
when it's within bounds.
Compatibility. Default limit is generous (1 MiB) and tunable per deployment;
existing in-bounds requests behave exactly as before. Setting
MaxRequestBodyBytes = 0restores the old unbounded behavior for anyone whoneeds it.
Tests added (
pkg/server/middleware_test.go):TestReadRequestBodyEnforcesLimit— a body exactly at the limit is acceptedand restored intact; one byte over is rejected with an
exceedserror.TestChargeMiddlewareRejectsOversizedBody— an oversized POST to a protectedroute returns
400before verification runs.go test ./pkg/server/→ok.