Add string replacement sanitizer to log injection

Author: owen-mc

go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll

@@ -41,22 +41,12 @@ module LogInjection {
   }
 
   /**
-   * A call to `strings.Replace` or `strings.ReplaceAll`, considered as a sanitizer
-   * for log injection.
+   * An expression that is equivalent to `strings.ReplaceAll(s, old, new)`,
+   * where `old` is a newline character, considered as a sanitizer for log
+   * injection.
    */
-  class ReplaceSanitizer extends Sanitizer {
-    ReplaceSanitizer() {
-      exists(string name, DataFlow::CallNode call |
-        this = call and
-        call.getTarget().hasQualifiedName("strings", name) and
-        call.getArgument(1).getStringValue().matches("%" + ["\r", "\n"] + "%")
-      |
-        name = "Replace" and
-        call.getArgument(3).getNumericValue() < 0
-        or
-        name = "ReplaceAll"
-      )
-    }
+  class ReplaceSanitizer extends StringOps::ReplaceAll, Sanitizer {
+    ReplaceSanitizer() { this.getReplacedString() = ["\r", "\n"] }
   }
 
   /**

go/ql/test/query-tests/Security/CWE-117/LogInjection.go

@@ -11,6 +11,7 @@ package main
 //go:generate depstubber -vendor go.uber.org/zap Logger,SugaredLogger NewProduction
 
 import (
+	"bytes"
 	"fmt"
 	"log"
 	"net/http"
@@ -378,8 +379,43 @@ func handlerGood2(req *http.Request) {
 	log.Printf("user %s logged in.\n", escapedUsername)
 }
 
+// GOOD: The user-provided value is escaped before being written to the log.
+func handlerGood3(req *http.Request) {
+	username := req.URL.Query()["username"][0]
+	replacer := strings.NewReplacer("\n", "", "\r", "")
+	log.Printf("user %s logged in.\n", replacer.Replace(username))
+	log.Printf("user %s logged in.\n", replacerLocal1(username))
+	log.Printf("user %s logged in.\n", replacerLocal2(username))
+	log.Printf("user %s logged in.\n", replacerGlobal1(username))
+	log.Printf("user %s logged in.\n", replacerGlobal2(username))
+}
+
+func replacerLocal1(s string) string {
+	replacer := strings.NewReplacer("\n", "", "\r", "")
+	return replacer.Replace(s)
+}
+
+func replacerLocal2(s string) string {
+	replacer := strings.NewReplacer("\n", "", "\r", "")
+	buf := new(bytes.Buffer)
+	replacer.WriteString(buf, s)
+	return buf.String()
+}
+
+var globalReplacer = strings.NewReplacer("\n", "", "\r", "")
+
+func replacerGlobal1(s string) string {
+	return globalReplacer.Replace(s)
+}
+
+func replacerGlobal2(s string) string {
+	buf := new(bytes.Buffer)
+	globalReplacer.WriteString(buf, s)
+	return buf.String()
+}
+
 // GOOD: User-provided values formatted using a %q directive, which escapes newlines
-func handlerGood3(req *http.Request, ctx *goproxy.ProxyCtx) {
+func handlerGood4(req *http.Request, ctx *goproxy.ProxyCtx) {
 	username := req.URL.Query()["username"][0]
 	testFlag := req.URL.Query()["testFlag"][0]
 	log.Printf("user %q logged in.\n", username)