Merge pull request #16931 from owen-mc/go/fix/clear-sanitizer

Go: fix `clear` sanitizer

Author: owen-mc

go/ql/lib/change-notes/2024-07-08-fix-clear-sanitizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* There was a bug which meant that the built-in function `clear` was considered as a sanitizer in some cases when it shouldn't have been. This has now been fixed, which may lead to more alerts.

go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll

@@ -423,7 +423,7 @@ private class ClearSanitizer extends DefaultTaintSanitizer {
       arg = call.getAnArgument() and
       arg = var.getAUse() and
       arg != this and
-      this.getBasicBlock().(ReachableBasicBlock).dominates(this.getBasicBlock())
+      arg.getBasicBlock().(ReachableBasicBlock).dominates(this.getBasicBlock())
     )
   }
 }

go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/Builtin.go

@@ -8,9 +8,38 @@ func clearTestBad(sourceReq *http.Request) string {
 	return string(b)
 }
 
+func clearTestBad2(sourceReq *http.Request, x bool) string {
+	b := make([]byte, 8)
+	sourceReq.Body.Read(b)
+	if x {
+		clear(b)
+	}
+	return string(b)
+}
+
+func clearTestBad3(sourceReq *http.Request, x bool) string {
+	b := make([]byte, 8)
+	sourceReq.Body.Read(b)
+	if x {
+		return string(b)
+	}
+	clear(b)
+	return string(b)
+}
+
 func clearTestGood(sourceReq *http.Request) string {
 	b := make([]byte, 8)
 	sourceReq.Body.Read(b)
 	clear(b) // should prevent taint flow
 	return string(b)
 }
+
+func clearTestGood2(sourceReq *http.Request, x bool) string {
+	b := make([]byte, 8)
+	sourceReq.Body.Read(b)
+	clear(b) // should prevent taint flow
+	if x {
+		return string(b)
+	}
+	return ""
+}

go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/DefaultSanitizer.expected

@@ -1,10 +1,22 @@
 edges
 | Builtin.go:6:2:6:2 | definition of b | Builtin.go:8:9:8:17 | type conversion | provenance |  |
 | Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | MaD:626 |
+| Builtin.go:12:2:12:2 | definition of b | Builtin.go:17:9:17:17 | type conversion | provenance |  |
+| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | MaD:626 |
+| Builtin.go:21:2:21:2 | definition of b | Builtin.go:24:10:24:18 | type conversion | provenance |  |
+| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | MaD:626 |
 nodes
 | Builtin.go:6:2:6:2 | definition of b | semmle.label | definition of b |
 | Builtin.go:7:2:7:15 | selection of Body | semmle.label | selection of Body |
 | Builtin.go:8:9:8:17 | type conversion | semmle.label | type conversion |
+| Builtin.go:12:2:12:2 | definition of b | semmle.label | definition of b |
+| Builtin.go:13:2:13:15 | selection of Body | semmle.label | selection of Body |
+| Builtin.go:17:9:17:17 | type conversion | semmle.label | type conversion |
+| Builtin.go:21:2:21:2 | definition of b | semmle.label | definition of b |
+| Builtin.go:22:2:22:15 | selection of Body | semmle.label | selection of Body |
+| Builtin.go:24:10:24:18 | type conversion | semmle.label | type conversion |
 subpaths
 #select
 | Builtin.go:8:9:8:17 | type conversion | Builtin.go:7:2:7:15 | selection of Body | Builtin.go:8:9:8:17 | type conversion | Found taint flow |
+| Builtin.go:17:9:17:17 | type conversion | Builtin.go:13:2:13:15 | selection of Body | Builtin.go:17:9:17:17 | type conversion | Found taint flow |
+| Builtin.go:24:10:24:18 | type conversion | Builtin.go:22:2:22:15 | selection of Body | Builtin.go:24:10:24:18 | type conversion | Found taint flow |