Merge branch 'main' into 17052-second-try-do-not-expose-error-message

java/ql/lib/change-notes/2024-07-24-url-fields-inherit-taint.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added flow through some methods of the class `java.net.URL` by ensuring that the fields of a URL are tainted.

java/ql/lib/ext/org.apache.commons.lang3.model.yml

@@ -3,6 +3,7 @@ extensions:
       pack: codeql/java-all
       extensible: sinkModel
     data:
+      # Note these sinks do not use the sink kind `regex-use[0]` because they should be considered as sinks for regex injection but not polynomial ReDoS.
       - ["org.apache.commons.lang3", "RegExUtils", False, "removeAll", "(String,String)", "", "Argument[1]", "regex-use", "manual"]
       - ["org.apache.commons.lang3", "RegExUtils", False, "removeFirst", "(String,String)", "", "Argument[1]", "regex-use", "manual"]
       - ["org.apache.commons.lang3", "RegExUtils", False, "removePattern", "(String,String)", "", "Argument[1]", "regex-use", "manual"]

java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll

@@ -22,6 +22,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.IoJsonWebToken
   private import semmle.code.java.frameworks.jackson.JacksonSerializability
   private import semmle.code.java.frameworks.InputStream
+  private import semmle.code.java.frameworks.Networking
   private import semmle.code.java.frameworks.Properties
   private import semmle.code.java.frameworks.Protobuf
   private import semmle.code.java.frameworks.ThreadLocal

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -356,8 +356,12 @@ RefType getErasedRepr(Type t) {
   t instanceof NullType and result instanceof TypeObject
 }
 
-class DataFlowType extends SrcRefType {
+final private class SrcRefTypeFinal = SrcRefType;
+
+class DataFlowType extends SrcRefTypeFinal {
   DataFlowType() { this = getErasedRepr(_) }
+
+  string toString() { result = ppReprType(this) }
 }
 
 pragma[nomagic]
@@ -371,7 +375,7 @@ DataFlowType getNodeType(Node n) {
 }
 
 /** Gets a string representation of a type returned by `getErasedRepr`. */
-string ppReprType(DataFlowType t) {
+private string ppReprType(SrcRefType t) {
   if t.(BoxedType).getPrimitiveType().getName() = "double"
   then result = "Number"
   else result = t.toString()

java/ql/lib/semmle/code/java/frameworks/Networking.qll

@@ -3,6 +3,8 @@
  */
 
 import semmle.code.java.Type
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.FlowSteps
 
 /** The type `java.net.URLConnection`. */
 class TypeUrlConnection extends RefType {
@@ -24,6 +26,11 @@ class TypeUrl extends RefType {
   TypeUrl() { this.hasQualifiedName("java.net", "URL") }
 }
 
+/** Specifies that if a `URL` is tainted, then so are its synthetic fields. */
+private class UrlFieldsInheritTaint extends DataFlow::SyntheticFieldContent, TaintInheritingContent {
+  UrlFieldsInheritTaint() { this.getField().matches("java.net.URL.%") }
+}
+
 /** The type `java.net.URLDecoder`. */
 class TypeUrlDecoder extends RefType {
   TypeUrlDecoder() { this.hasQualifiedName("java.net", "URLDecoder") }

java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll

@@ -13,9 +13,15 @@ private class ExploitableStringLiteral extends StringLiteral {
 
 /**
  * Holds if `kind` is an external sink kind that is relevant for regex flow.
- * `full` is true if sinks with this kind match against the full string of its input.
- * `strArg` is the index of the argument to methods with this sink kind that contan the string to be matched against,
- * where -1 is the qualifier; or -2 if no such argument exists.
+ * `full` is true if sinks with this kind match against the full string of its
+ * input.
+ * `strArg` is the index of the argument to methods with this sink kind that
+ * contain the string to be matched against, where -1 is the qualifier; or -2
+ * if no such argument exists.
+ *
+ * Note that `regex-use` is deliberately not a possible value for `kind` here,
+ * as it is used for regular expression injection sinks that should not be used
+ * as polynomial ReDoS sinks.
  */
 private predicate regexSinkKindInfo(string kind, boolean full, int strArg) {
   sinkModel(_, _, _, _, _, _, _, kind, _, _) and

java/ql/test/library-tests/frameworks/jdk/java.net/Test.java

@@ -90,6 +90,14 @@ public class Test {
 			out = in.toURL();
 			sink(out); // $ hasTaintFlow
 		}
+		{
+			// manual test for `URI.toURL().getPath()`; checks that if a `URL` is tainted, then so are its synthetic fields
+			// java.net;URL;False;getPath;();;Argument[this].SyntheticField[java.net.URL.path];ReturnValue;taint;ai-manual
+			URL out = null;
+			URI in = (URI) source();
+			out = in.toURL();
+			sink(out.getPath()); // $ hasTaintFlow
+		}
 		{
 			// "java.net;URL;false;URL;(String);;Argument[0];Argument[this];taint;manual"
 			URL out = null;
@@ -97,6 +105,14 @@ public class Test {
 			out = new URL(in);
 			sink(out); // $ hasTaintFlow
 		}
+		{
+			// manual test for `URL(String).getPath()`; checks that if a `URL` is tainted, then so are its synthetic fields
+			// java.net;URL;False;getPath;();;Argument[this].SyntheticField[java.net.URL.path];ReturnValue;taint;ai-manual
+			URL out = null;
+			String in = (String) source();
+			out = new URL(in);
+			sink(out.getPath()); // $ hasTaintFlow
+		}
 		{
 			// "java.net;URL;false;URL;(URL,String);;Argument[0];Argument[this];taint;ai-generated"
 			URL out = null;