add inline tests for open redirect,xss, fix some issues in fasthttp.qll

2026-04-29 02:35:15 +02:00 · 2023-10-12 08:39:21 +02:00
parent 29219922ac
commit 3bc24c3534
7 changed files with 98 additions and 77 deletions
--- a/go/ql/lib/semmle/go/frameworks/Fasthttp.qll
+++ b/go/ql/lib/semmle/go/frameworks/Fasthttp.qll
@@ -343,6 +343,42 @@ module Fasthttp {
    }
  }

+  /**
+   * Provide modeling for fasthttp.Response Type
+   */
+  module Response {
+    /**
+     * A Method That send files from its input and it does not check input path against path traversal attacks, so it is a dangerous method
+     */
+    class FileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+      FileSystemAccess() {
+        exists(Method mcn |
+          mcn.hasQualifiedName(packagePath(), "Response", "SendFile") and
+          this = mcn.getACall()
+        )
+      }
+
+      override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
+    }
+
+    /**
+     * The methods that can write to HTTP Response Body.
+     * These methods can be dangerous if they are user controllable.
+     */
+    class HttpResponseBodySink extends SharedXss::Sink {
+      HttpResponseBodySink() {
+        exists(Method m |
+          m.hasQualifiedName(packagePath(), "Response",
+            [
+              "AppendBody", "AppendBodyString", "SetBody", "SetBodyString", "SetBodyRaw",
+              "SetBodyStream"
+            ]) and
+          this = m.getACall().getArgument(0)
+        )
+      }
+    }
+  }
+
  /**
   * Provide modeling for fasthttp.Request Type
   */
@@ -371,42 +407,6 @@ module Fasthttp {
      }
    }

-    /**
-     * Provide modeling for fasthttp.Response Type
-     */
-    module Response {
-      /**
-       * A Method That send files from its input and it does not check input path against path traversal attacks, so it is a dangerous method
-       */
-      class FileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
-        FileSystemAccess() {
-          exists(Method mcn |
-            mcn.hasQualifiedName(packagePath(), "Response", "SendFile") and
-            this = mcn.getACall()
-          )
-        }
-
-        override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-      }
-
-      /**
-       * The methods that can write to HTTP Response Body.
-       * These methods can be dangerous if they are user controllable.
-       */
-      class HttpResponseBodySink extends SharedXss::Sink {
-        HttpResponseBodySink() {
-          exists(Method m |
-            m.hasQualifiedName(packagePath(), "Response",
-              [
-                "AppendBody", "AppendBodyString", "SetBody", "SetBodyString", "SetBodyRaw",
-                "SetBodyStream"
-              ]) and
-            this = m.getACall().getArgument(0)
-          )
-        }
-      }
-    }
-
    /**
     * The methods as Remote user controllable source which can be many part of request
     */
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/OpenRedirect.expected
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/OpenRedirect.expected
@@ -1,2 +1,2 @@
-| fasthttp.go:218:23:218:50 | "https://userControlled.com" |
-| fasthttp.go:219:28:219:63 | type conversion |
+testFailures
+failures
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/OpenRedirect.ql
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/OpenRedirect.ql
@@ -1,4 +1,19 @@
 import go
 import semmle.go.security.OpenUrlRedirectCustomizations
+import TestUtilities.InlineExpectationsTest

-select any(OpenUrlRedirect::Sink s)
+module FasthttpTest implements TestSig {
+  string getARelevantTag() { result = "OpenRedirect" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(OpenUrlRedirect::Sink s |
+      s.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = s.toString() and
+      value =  s.toString() and
+      tag = "OpenRedirect"
+    )
+  }
+}
+
+import MakeTest<FasthttpTest>
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/UntrustedRemoteFlowSource.ql
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/UntrustedRemoteFlowSource.ql
@@ -5,11 +5,11 @@ module FasthttpTest implements TestSig {
  string getARelevantTag() { result = "UntrustedFlowSource" }

  predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(UntrustedFlowSource q |
-      q.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+    exists(UntrustedFlowSource source |
+      source.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
-      element = q.toString() and
-      value = "\"" + q.toString() + "\"" and
+      element = source.toString() and
+      value = "\"" + source.toString() + "\"" and
      tag = "UntrustedFlowSource"
    )
  }
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/Xss.expected
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/Xss.expected
@@ -1,11 +1,2 @@
-| fasthttp.go:194:34:194:58 | type conversion |
-| fasthttp.go:195:40:195:56 | "user Controlled" |
-| fasthttp.go:198:31:198:55 | type conversion |
-| fasthttp.go:199:37:199:53 | "user Controlled" |
-| fasthttp.go:200:34:200:58 | type conversion |
-| fasthttp.go:201:37:201:45 | dstReader |
-| fasthttp.go:207:26:207:39 | type conversion |
-| fasthttp.go:208:32:208:37 | "body" |
-| fasthttp.go:213:34:213:90 | call to AppendQuotedArg |
-| fasthttp.go:214:34:214:83 | call to AppendHTMLEscape |
-| fasthttp.go:215:34:215:96 | call to AppendHTMLEscapeBytes |
+testFailures
+failures
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/Xss.ql
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/Xss.ql
@@ -1,3 +1,19 @@
 import go
+import TestUtilities.InlineExpectationsTest

-select any(SharedXss::Sink s)
+module FasthttpTest implements TestSig {
+  string getARelevantTag() { result = "XssSink" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(SharedXss::Sink xssSink |
+      xssSink
+          .hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+            location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = xssSink.toString() and
+      value = xssSink.toString() and
+      tag = "XssSink"
+    )
+  }
+}
+
+import MakeTest<FasthttpTest>
--- a/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/fasthttp.go
+++ b/go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/fasthttp.go
@@ -22,7 +22,6 @@ func fasthttpClient() {
 	res := &fasthttp.Response{}
 	req := &fasthttp.Request{}
 	uri := fasthttp.AcquireURI()
-	uri2 := fasthttp.AcquireURI()
 	fasthttp.Get(resByte, "http://127.0.0.1:8909")                      // $ SSRF="http://127.0.0.1:8909"
 	fasthttp.GetDeadline(resByte, "http://127.0.0.1:8909", time.Time{}) // $ SSRF="http://127.0.0.1:8909"
 	fasthttp.GetTimeout(resByte, "http://127.0.0.1:8909", 5)            // $ SSRF="http://127.0.0.1:8909"
@@ -174,35 +173,35 @@ func fasthttpServer() {

 		// Response methods
 		// Xss Sinks Related method
-		requestCtx.Response.AppendBody([]byte("user Controlled")) // $ XSS=[]byte("user Controlled")
-		requestCtx.Response.AppendBodyString("user Controlled")   // $ XSS="user Controlled"
-		rspWriter := requestCtx.Response.BodyWriter()
-		rspWriter.Write([]byte("XSS"))                            // $ XSS=[]byte("XSS")
-		requestCtx.Response.SetBody([]byte("user Controlled"))    // $ XSS=[]byte("XSS")
-		requestCtx.Response.SetBodyString("user Controlled")      // $ XSS=[]byte("XSS")
-		requestCtx.Response.SetBodyRaw([]byte("user Controlled")) // $ XSS=[]byte("XSS")
-		requestCtx.Response.SetBodyStream(dstReader, 100)         // $ XSS=[]byte("XSS")
+		userInput := "user Controlled input"
+		userInputByte := []byte("user Controlled input")
+		requestCtx.Response.AppendBody(userInputByte)   // $ XssSink=userInputByte
+		requestCtx.Response.AppendBodyString(userInput) // $ XssSink=userInput
+		rspWriter := requestCtx.Response.BodyWriter()   // IDK how to handle this that returns a `io.Writer`
+		rspWriter.Write(userInputByte)
+		requestCtx.Response.SetBody(userInputByte)        // $ XssSink=userInputByte
+		requestCtx.Response.SetBodyString(userInput)      // $ XssSink=userInput
+		requestCtx.Response.SetBodyRaw(userInputByte)     // $ XssSink=userInputByte
+		requestCtx.Response.SetBodyStream(dstReader, 100) // $ XssSink=dstReader
 		// mostly related to header writers
-		requestCtx.Response.Header.Set("Content-Type", "")
-		requestCtx.Response.Header.Add("Content-Type", "")
-		requestCtx.Response.Header.SetContentTypeBytes([]byte(""))
-		requestCtx.Response.Header.SetContentType("")
-		requestCtx.Success("", []byte("body")) // $ XSS=[]byte("body")
-		requestCtx.SuccessString("", "body")   // $ XSS="body"
-		requestCtx.SetContentType("")
-		requestCtx.SetContentTypeBytes([]byte(""))
+		requestCtx.Success("", userInputByte)   // $ XssSink=userInputByte
+		requestCtx.SuccessString("", userInput) // $ XssSink=userInput

 		// sanitizers
-		requestCtx.Response.AppendBody(fasthttp.AppendQuotedArg([]byte(""), []byte("<>\"':()&"))) // $ Sanitizer=AppendBody
+		userInputByte = []byte("<>\"':()&")
+		userInput = "<>\"':()&"
+		fasthttp.AppendQuotedArg([]byte(""), userInputByte) // $ Sanitizer=userInputByte
 		// %3C%3E%22%27%3A%28%29%26
-		requestCtx.Response.AppendBody(fasthttp.AppendHTMLEscape([]byte(""), "<>\"':()&")) // $ Sanitizer=AppendBody
+		fasthttp.AppendHTMLEscape([]byte(""), userInput) // $ Sanitizer=userInput
 		// &lt;&gt;&#34;&#39;:()&amp;
-		requestCtx.Response.AppendBody(fasthttp.AppendHTMLEscapeBytes([]byte(""), []byte("<>\"':()&"))) // $ Sanitizer=AppendBody
+		fasthttp.AppendHTMLEscapeBytes([]byte(""), userInputByte) // $ Sanitizer=userInputByte
 		// &lt;&gt;&#34;&#39;:()&amp;

 		// open redirect Sinks
-		requestCtx.Redirect("https://userControlled.com", 301)              // $ OpenRedirect="https://userControlled.com"
-		requestCtx.RedirectBytes([]byte("https://userControlled.com"), 301) // $ OpenRedirect=[]byte("https://userControlled.com")
+		userInput = "https://userControlled.com"
+		requestCtx.Redirect(userInput, 301) // $ OpenRedirect=userInput
+		userInputByte = []byte("https://userControlled.com")
+		requestCtx.RedirectBytes(userInputByte, 301) // $ OpenRedirect=userInputByte
 	}
 	fasthttp.Serve(ln, requestHandler)
 }