Skip to content

Commit

Permalink
r.ParseMultipartForm and io.ReadAll update
Browse files Browse the repository at this point in the history
It's not super serious, but caught my eye.
  • Loading branch information
bcaller committed Feb 29, 2024
1 parent 1ea1f62 commit a2865c6
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 5 deletions.
17 changes: 17 additions & 0 deletions assets/semgrep_rules/services/http-parse-multipart-dos.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
func handleBad(w http.ResponseWriter, r *http.Request) error {
// ruleid: http-parse-multipart-dos
if err = r.ParseMultipartForm(maxSize); err != nil {
return err
}
return nil
}

func handleOK(w http.ResponseWriter, r *http.Request) error {
r.Body = http.MaxBytesReader(w, r.Body, 123)
fmt.Print("banana")
// ok: http-parse-multipart-dos
if err = r.ParseMultipartForm(maxSize); err != nil {
return err
}
return nil
}
26 changes: 26 additions & 0 deletions assets/semgrep_rules/services/http-parse-multipart-dos.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
rules:
- id: http-parse-multipart-dos
metadata:
author: Ben Caller
confidence: LOW
references:
- https://pkg.go.dev/net/http#Request.ParseMultipartForm
- https://pkg.go.dev/net/http#MaxBytesReader
source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/services/http-parse-multipart-dos.yaml
assignees: |
bcaller
thypon
severity: INFO
languages:
- go
patterns:
- pattern: $R.ParseMultipartForm($MAXSIZE)
- pattern-not-inside: |
$R.Body = http.MaxBytesReader($W, <...$R.Body...>, $LIMIT)
...
fix: $R.Body = http.MaxBytesReader($W, $R.Body, $MAXSIZE)
message: |
ParseMultipartForm is vulnerable to Denial of Service (DoS) by clients sending a large HTTP request body.
The specified limit of $MAXSIZE is the maximum amount stored in memory.
The remainder is still parsed and stored on disk in temporary files.
Wrapping $R.Body with http.MaxBytesReader prevents this wasting of server resources.
17 changes: 14 additions & 3 deletions assets/semgrep_rules/services/io-readall-dos.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
// ruleid: io-readall-dos
io.ReadAll(r.Body)
func handleBad(w http.ResponseWriter, r *http.Request) []byte {
// ruleid: io-readall-dos
payload, _ = io.ReadAll(r.Body)
return payload
}

func handleOK(w http.ResponseWriter, r *http.Request) []byte {
r.Body = http.MaxBytesReader(w, r.Body, 123)
fmt.Print("banana")
// ok: io-readall-dos
payload, _ = io.ReadAll(r.Body)
return payload
}

// ok: io-readall-dos
io.ReadAll(http.MaxBytesReader(w, r.Body, u.maxRequestSize))
io.ReadAll(io.LimitReader(r.Body, u.maxRequestSize))

// ok: io-readall-dos
io.ReadAll(x.y)
8 changes: 6 additions & 2 deletions assets/semgrep_rules/services/io-readall-dos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ rules:
severity: INFO
languages:
- go
pattern: io.ReadAll($R.Body)
patterns:
- pattern: io.ReadAll($R.Body)
- pattern-not-inside: |
$R.Body = http.MaxBytesReader($W, <...$R.Body...>, $LIMIT)
...
fix: io.ReadAll(http.MaxBytesReader(w, $R.Body, MAX_REQUEST_SIZE))
message: |
io.ReadAll is vulnerable to Denial of Service (DoS) by clients sending a large HTTP request body.
Wrapping $R.Body with http.MaxBytesReader prevents this wasting of server resources.
Wrapping $R.Body with http.MaxBytesReader (or io.LimitReader) prevents this wasting of server resources.

0 comments on commit a2865c6

Please sign in to comment.