Skip to content

Commit

Permalink
Be more defensive with the response body:
Browse files Browse the repository at this point in the history
Moving the response body copy to after the
content length check and only coping the
resp.Contentlegnth should give us more
defense against malicious actors.

Signed-off-by: Jacob Weinstock <[email protected]>
  • Loading branch information
jacobweinstock committed Sep 13, 2023
1 parent 6440d17 commit 61c88e3
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions providers/rpc/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,15 @@ func (p *Provider) process(ctx context.Context, rp RequestPayload) (ResponsePayl
return ResponsePayload{}, err
}
defer resp.Body.Close()
respBuf := new(bytes.Buffer)
if _, err := io.Copy(respBuf, resp.Body); err != nil {
return ResponsePayload{}, fmt.Errorf("failed to read response body: %w", err)
}

// handle the response
if resp.ContentLength > maxContentLenAllowed || resp.ContentLength < 0 {
return ResponsePayload{}, fmt.Errorf("response body is too large: %d bytes, max allowed: %d bytes", resp.ContentLength, maxContentLenAllowed)

Check warning on line 339 in providers/rpc/rpc.go

View check run for this annotation

Codecov / codecov/patch

providers/rpc/rpc.go#L339

Added line #L339 was not covered by tests
}
respBuf := new(bytes.Buffer)
if _, err := io.CopyN(respBuf, resp.Body, resp.ContentLength); err != nil {
return ResponsePayload{}, fmt.Errorf("failed to read response body: %w", err)

Check warning on line 343 in providers/rpc/rpc.go

View check run for this annotation

Codecov / codecov/patch

providers/rpc/rpc.go#L343

Added line #L343 was not covered by tests
}
respPayload, err := p.handleResponse(resp.StatusCode, resp.Header, respBuf, kvs)
if err != nil {
return ResponsePayload{}, err

Check warning on line 347 in providers/rpc/rpc.go

View check run for this annotation

Codecov / codecov/patch

providers/rpc/rpc.go#L347

Added line #L347 was not covered by tests
Expand Down

0 comments on commit 61c88e3

Please sign in to comment.