Java: Report log-injection at the source rather than the sink

smowton · smowton · commit 1f9f6d7c3342 · 2022-06-22T13:05:20.000+01:00
This should remove the problem of excessive grouping of different alerts that share a sink location, often due to wrapper functions that form the ultimate sink of all logging calls in a given codebase.
diff --git a/java/ql/src/Security/CWE/CWE-117/LogInjection.ql b/java/ql/src/Security/CWE/CWE-117/LogInjection.ql
@@ -17,5 +17,5 @@ import DataFlow::PathGraph
 
 from LogInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "This $@ flows to a log entry.", source.getNode(),
-  "user-provided value"
+select source.getNode(), source, sink, "This user-provided value flows to a $@.", sink.getNode(),
+  "log entry"
diff --git a/java/ql/src/change-notes/2022-06-22-log-injection-location.md b/java/ql/src/change-notes/2022-06-22-log-injection-location.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `java/log-injection` now reports problems at the source (user-controlled data) instead of at the ultimate logging call. This was changed because user functions that wrap the ultimate logging call could result in most alerts being reported in an uninformative location.

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* The query `java/log-injection` now reports problems at the source (user-controlled data) instead of at the ultimate logging call. This was changed because user functions that wrap the ultimate logging call could result in most alerts being reported in an uninformative location.