Add use of strings.Replacer to replace sanitizer

Author: owen-mc

go/ql/lib/semmle/go/StringOps.qll

@@ -206,6 +206,68 @@ module StringOps {
 
       override string getReplacedString() { result = this.getArgument(1).getStringValue() }
     }
+
+    /**
+     * A call to `strings.NewReplacer`.
+     */
+    private class StringsNewReplacerCall extends DataFlow::CallNode {
+      StringsNewReplacerCall() { this.getTarget().hasQualifiedName("strings", "NewReplacer") }
+
+      /**
+       * Gets an argument to this call corresponding to a string that will be
+       * replaced.
+       */
+      DataFlow::Node getAReplacedArgument() {
+        exists(int n | n % 2 = 0 and result = this.getArgument(n))
+      }
+    }
+
+    /**
+     * A configuration for tracking flow from a call to `strings.NewReplacer` to
+     * the receiver of a call to `strings.Replacer.Replace` or
+     * `strings.Replacer.WriteString`.
+     */
+    private class StringsNewReplacerConfiguration extends DataFlow2::Configuration {
+      StringsNewReplacerConfiguration() { this = "StringsNewReplacerConfiguration" }
+
+      override predicate isSource(DataFlow::Node source) {
+        source instanceof StringsNewReplacerCall
+      }
+
+      override predicate isSink(DataFlow::Node sink) {
+        exists(DataFlow::MethodCallNode call |
+          sink = call.getReceiver() and
+          call.getTarget().hasQualifiedName("strings", "Replacer", ["Replace", "WriteString"])
+        )
+      }
+    }
+
+    /**
+     * A call to `strings.Replacer.Replace` or `strings.Replacer.WriteString`.
+     */
+    private class StringsReplacerReplaceOrWriteString extends Range {
+      string replacedString;
+
+      StringsReplacerReplaceOrWriteString() {
+        exists(
+          StringsNewReplacerConfiguration config, StringsNewReplacerCall source,
+          DataFlow::Node sink, DataFlow::MethodCallNode call
+        |
+          config.hasFlow(source, sink) and
+          sink = call.getReceiver() and
+          replacedString = source.getAReplacedArgument().getStringValue() and
+          (
+            call.getTarget().hasQualifiedName("strings", "Replacer", "Replace") and
+            this = call.getResult()
+            or
+            call.getTarget().hasQualifiedName("strings", "Replacer", "WriteString") and
+            this = call.getArgument(1)
+          )
+        )
+      }
+
+      override string getReplacedString() { result = replacedString }
+    }
   }
 
   /** Provides predicates and classes for working with Printf-style formatters. */

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

@@ -87,7 +87,7 @@ module StringBreak {
    * An expression that is equivalent to `strings.ReplaceAll(s, old, new)`,
    * considered as a sanitizer for unsafe quoting.
    */
-  class ReplaceSanitizer extends Sanitizer, StringOps::ReplaceAll {
+  class ReplaceSanitizer extends StringOps::ReplaceAll, Sanitizer {
     Quote quote;
 
     ReplaceSanitizer() { this.getReplacedString().matches("%" + quote + "%") }

go/ql/test/query-tests/Security/CWE-089/StringBreakGood.go

@@ -1,9 +1,11 @@
 package main
 
 import (
+	"bytes"
 	"encoding/json"
-	sq "github.com/Masterminds/squirrel"
 	"strings"
+
+	sq "github.com/Masterminds/squirrel"
 )
 
 // Good because there is no concatenation with quotes:
@@ -37,3 +39,28 @@ func saveGood3(id string, version interface{}) {
 		Values(id, sq.Expr("'"+escaped+"'")).
 		Exec()
 }
+
+var globalReplacer = strings.NewReplacer("\"", "", "'", "")
+
+// Good because quote characters are removed before concatenation:
+func saveGood4(id string, version interface{}) {
+	versionJSON, _ := json.Marshal(version)
+	escaped := globalReplacer.Replace(string(versionJSON))
+	sq.StatementBuilder.
+		Insert("resources").
+		Columns("resource_id", "version_md5").
+		Values(id, sq.Expr("'"+escaped+"'")).
+		Exec()
+}
+
+// Good because quote characters are removed before concatenation:
+func saveGood5(id string, version interface{}) {
+	versionJSON, _ := json.Marshal(version)
+	buf := new(bytes.Buffer)
+	globalReplacer.WriteString(buf, string(versionJSON))
+	sq.StatementBuilder.
+		Insert("resources").
+		Columns("resource_id", "version_md5").
+		Values(id, sq.Expr("'"+buf.String()+"'")).
+		Exec()
+}