From a2865c634d30127370520816fc65efa409e54d29 Mon Sep 17 00:00:00 2001
From: bcaller <bcaller@brave.com>
Date: Thu, 29 Feb 2024 11:51:47 +0800
Subject: [PATCH] r.ParseMultipartForm and io.ReadAll update

It's not super serious, but caught my eye.
---
 .../services/http-parse-multipart-dos.go      | 17 ++++++++++++
 .../services/http-parse-multipart-dos.yaml    | 26 +++++++++++++++++++
 .../semgrep_rules/services/io-readall-dos.go  | 17 +++++++++---
 .../services/io-readall-dos.yaml              |  8 ++++--
 4 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 assets/semgrep_rules/services/http-parse-multipart-dos.go
 create mode 100644 assets/semgrep_rules/services/http-parse-multipart-dos.yaml

diff --git a/assets/semgrep_rules/services/http-parse-multipart-dos.go b/assets/semgrep_rules/services/http-parse-multipart-dos.go
new file mode 100644
index 00000000..87a5342f
--- /dev/null
+++ b/assets/semgrep_rules/services/http-parse-multipart-dos.go
@@ -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
+}
\ No newline at end of file
diff --git a/assets/semgrep_rules/services/http-parse-multipart-dos.yaml b/assets/semgrep_rules/services/http-parse-multipart-dos.yaml
new file mode 100644
index 00000000..b204b8b7
--- /dev/null
+++ b/assets/semgrep_rules/services/http-parse-multipart-dos.yaml
@@ -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.
\ No newline at end of file
diff --git a/assets/semgrep_rules/services/io-readall-dos.go b/assets/semgrep_rules/services/io-readall-dos.go
index b6ea1edf..7f446e06 100644
--- a/assets/semgrep_rules/services/io-readall-dos.go
+++ b/assets/semgrep_rules/services/io-readall-dos.go
@@ -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)
\ No newline at end of file
diff --git a/assets/semgrep_rules/services/io-readall-dos.yaml b/assets/semgrep_rules/services/io-readall-dos.yaml
index 6114e7de..a9780e9c 100644
--- a/assets/semgrep_rules/services/io-readall-dos.yaml
+++ b/assets/semgrep_rules/services/io-readall-dos.yaml
@@ -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.
\ No newline at end of file
+        Wrapping $R.Body with http.MaxBytesReader (or io.LimitReader) prevents this wasting of server resources.
\ No newline at end of file