Add extra replace sanitizers to StringBreak

Author: owen-mc

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

@@ -105,4 +105,67 @@ module StringBreak {
 
     override Quote getQuote() { result = quote }
   }
+
+  class StringsNewReplacerCall extends DataFlow::CallNode {
+    StringsNewReplacerCall() { this.getTarget().hasQualifiedName("strings", "NewReplacer") }
+
+    DataFlow::Node getAReplacedArgument() {
+      exists(int m, int n | m = 2 * n and n = m / 2 and result = getArgument(m))
+    }
+  }
+
+  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`, considered as a sanitizer for unsafe
+   * quoting.
+   */
+  class ReplacerReplaceSanitizer extends DataFlow::MethodCallNode, Sanitizer {
+    Quote quote;
+
+    ReplacerReplaceSanitizer() {
+      exists(StringsNewReplacerConfiguration config, DataFlow::Node source, DataFlow::Node sink |
+        config.hasFlow(source, sink) and
+        this.getTarget().hasQualifiedName("strings", "Replacer", "Replace") and
+        sink = this.getReceiver() and
+        quote = source.(StringsNewReplacerCall).getAReplacedArgument().getStringValue()
+      )
+    }
+
+    override Quote getQuote() { result = quote }
+  }
+
+  /**
+   * A call to `strings.Replacer.WriteString`, considered as a sanitizer for
+   * unsafe quoting.
+   */
+  class ReplacerWriteStringSanitizer extends Sanitizer {
+    Quote quote;
+
+    ReplacerWriteStringSanitizer() {
+      exists(
+        StringsNewReplacerConfiguration config, DataFlow::Node source, DataFlow::Node sink,
+        DataFlow::MethodCallNode call
+      |
+        config.hasFlow(source, sink) and
+        call.getTarget().hasQualifiedName("strings", "Replacer", "WriteString") and
+        sink = call.getReceiver() and
+        this = call.getArgument(1) and
+        quote = source.(StringsNewReplacerCall).getAReplacedArgument().getStringValue()
+      )
+    }
+
+    override Quote getQuote() { result = 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()
+}