Java: make numeric flow models neutral

Jami Cogswell · Jami Cogswell · commit 6bb865ad0511 · 2023-01-11T18:04:43.000-05:00
diff --git a/java/ql/lib/ext/java.lang.model.yml b/java/ql/lib/ext/java.lang.model.yml
@@ -41,17 +41,9 @@ extensions:
       - ["java.lang", "IllegalArgumentException", False, "IllegalArgumentException", "(String)", "", "Argument[0]", "Argument[-1].SyntheticField[java.lang.Throwable.message]", "value", "manual"]
       - ["java.lang", "IllegalStateException", False, "IllegalStateException", "(String)", "", "Argument[0]", "Argument[-1].SyntheticField[java.lang.Throwable.message]", "value", "manual"]
       - ["java.lang", "IndexOutOfBoundsException", False, "IndexOutOfBoundsException", "(String)", "", "Argument[0]", "Argument[-1].SyntheticField[java.lang.Throwable.message]", "value", "manual"]
-      - ["java.lang", "Integer", False, "intValue", "()", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Integer", False, "parseInt", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Integer", False, "toString", "(int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Integer", False, "valueOf", "(int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["java.lang", "Iterable", True, "forEach", "(Consumer)", "", "Argument[-1].Element", "Argument[0].Parameter[0]", "value", "manual"]
       - ["java.lang", "Iterable", True, "iterator", "()", "", "Argument[-1].Element", "ReturnValue.Element", "value", "manual"]
       - ["java.lang", "Iterable", True, "spliterator", "()", "", "Argument[-1].Element", "ReturnValue.Element", "value", "manual"]
-      - ["java.lang", "Long", False, "longValue", "()", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Long", False, "parseLong", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Long", False, "toString", "()", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "Math", False, "min", "(int,int)", "", "Argument[0..1]", "ReturnValue", "value", "manual"]
       - ["java.lang", "Object", True, "clone", "", "", "Argument[-1].Element", "ReturnValue.Element", "value", "manual"]
       - ["java.lang", "Object", True, "clone", "", "", "Argument[-1].MapKey", "ReturnValue.MapKey", "value", "manual"]
       - ["java.lang", "Object", True, "clone", "", "", "Argument[-1].MapValue", "ReturnValue.MapValue", "value", "manual"]
@@ -97,8 +89,6 @@ extensions:
       - ["java.lang", "String", False, "valueOf", "(char)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["java.lang", "String", False, "valueOf", "(char[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["java.lang", "String", False, "valueOf", "(char[],int,int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "String", False, "valueOf", "(int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.lang", "String", False, "valueOf", "(long)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["java.lang", "StringBuffer", True, "StringBuffer", "(CharSequence)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
       - ["java.lang", "StringBuffer", True, "StringBuffer", "(String)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
       - ["java.lang", "StringBuilder", True, "StringBuilder", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
@@ -143,3 +133,16 @@ extensions:
       - ["java.lang", "System", "nanoTime", "()", "manual"]
       - ["java.lang", "Thread", "currentThread", "()", "manual"]
       - ["java.lang", "Thread", "sleep", "(long)", "manual"]
+
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.lang", "Integer", "intValue", "()", "manual"]       # taint-numeric
+      - ["java.lang", "Integer", "parseInt", "(String)", "manual"] # taint-numeric
+      - ["java.lang", "Integer", "toString", "(int)", "manual"]    # taint-numeric
+      - ["java.lang", "Integer", "valueOf", "(int)", "manual"]     # taint-numeric
+      - ["java.lang", "Long", "longValue", "()", "manual"]         # taint-numeric
+      - ["java.lang", "Long", "parseLong", "(String)", "manual"]   # taint-numeric
+      - ["java.lang", "Long", "toString", "()", "manual"]          # taint-numeric
+      - ["java.lang", "Math", "min", "(int,int)", "manual"]        # value-numeric
+      - ["java.lang", "String", "valueOf", "(int)", "manual"]      # taint-numeric
+      - ["java.lang", "String", "valueOf", "(long)", "manual"]     # taint-numeric
diff --git a/java/ql/lib/ext/java.math.model.yml b/java/ql/lib/ext/java.math.model.yml
@@ -1,14 +1,12 @@
 extensions:
-  - addsTo:
-      pack: codeql/java-all
-      extensible: summaryModel
-    data:
-      - ["java.math", "BigDecimal", False, "BigDecimal", "(String)", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
-      - ["java.math", "BigDecimal", False, "valueOf", "(double)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-      - ["java.math", "BigDecimal", False, "valueOf", "(long)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
-
   - addsTo:
       pack: codeql/java-all
       extensible: neutralModel
     data:
       - ["java.math", "BigDecimal", "compareTo", "(BigDecimal)", "manual"]
+
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.math", "BigDecimal", "BigDecimal", "(String)", "manual"] # taint-numeric
+      - ["java.math", "BigDecimal", "valueOf", "(double)", "manual"]    # taint-numeric
+      - ["java.math", "BigDecimal", "valueOf", "(long)", "manual"]      # taint-numeric
diff --git a/java/ql/lib/ext/java.sql.model.yml b/java/ql/lib/ext/java.sql.model.yml
@@ -19,13 +19,16 @@ extensions:
       pack: codeql/java-all
       extensible: summaryModel
     data:
-      - ["java.sql", "PreparedStatement", True, "setInt", "(int,int)", "", "Argument[1]", "Argument[-1]", "value", "manual"]
       - ["java.sql", "PreparedStatement", True, "setString", "(int,String)", "", "Argument[1]", "Argument[-1]", "value", "manual"]
-      - ["java.sql", "ResultSet", True, "getInt", "(String)", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
       - ["java.sql", "ResultSet", True, "getString", "(String)", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
 
   - addsTo:
       pack: codeql/java-all
       extensible: neutralModel
     data:
       - ["java.sql", "ResultSet", "next", "()", "manual"]
+
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.sql", "PreparedStatement", "setInt", "(int,int)", "manual"] # value-numeric
+      - ["java.sql", "ResultSet", "getInt", "(String)", "manual"]          # taint-numeric
diff --git a/java/ql/lib/ext/java.util.concurrent.atomic.model.yml b/java/ql/lib/ext/java.util.concurrent.atomic.model.yml
@@ -3,7 +3,14 @@ extensions:
       pack: codeql/java-all
       extensible: summaryModel
     data:
-      - ["java.util.concurrent.atomic", "AtomicInteger", False, "AtomicInteger", "(int)", "", "Argument[0]", "Argument[-1].SyntheticField[java.util.concurrent.atomic.AtomicInteger.value]", "value", "manual"]
-      - ["java.util.concurrent.atomic", "AtomicInteger", False, "get", "()", "", "Argument[-1].SyntheticField[java.util.concurrent.atomic.AtomicInteger.value]", "ReturnValue", "value", "manual"]
       - ["java.util.concurrent.atomic", "AtomicReference", False, "AtomicReference", "(Object)", "", "Argument[0]", "Argument[-1].SyntheticField[java.util.concurrent.atomic.AtomicReference.value]", "value", "manual"]
       - ["java.util.concurrent.atomic", "AtomicReference", False, "get", "()", "", "Argument[-1].SyntheticField[java.util.concurrent.atomic.AtomicReference.value]", "ReturnValue", "value", "manual"]
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.util.concurrent.atomic", "AtomicInteger", "AtomicInteger", "(int)", "manual"] # value-numeric
+      - ["java.util.concurrent.atomic", "AtomicInteger", "get", "()", "manual"]              # value-numeric
diff --git a/java/ql/lib/ext/java.util.concurrent.model.yml b/java/ql/lib/ext/java.util.concurrent.model.yml
@@ -18,8 +18,6 @@ extensions:
       - ["java.util.concurrent", "BlockingQueue", True, "put", "(Object)", "", "Argument[0]", "Argument[-1].Element", "value", "manual"]
       - ["java.util.concurrent", "BlockingQueue", True, "take", "()", "", "Argument[-1].Element", "ReturnValue", "value", "manual"]
       - ["java.util.concurrent", "ConcurrentHashMap", True, "elements", "()", "", "Argument[-1].MapValue", "ReturnValue.Element", "value", "manual"]
-      - ["java.util.concurrent", "CountDownLatch", False, "CountDownLatch", "(int)", "", "Argument[0]", "Argument[-1].SyntheticField[java.util.concurrent.CountDownLatch.count]", "value", "manual"]
-      - ["java.util.concurrent", "CountDownLatch", False, "getCount", "()", "", "Argument[-1].SyntheticField[java.util.concurrent.CountDownLatch.count]", "ReturnValue", "value", "manual"]
       - ["java.util.concurrent", "TransferQueue", True, "transfer", "(Object)", "", "Argument[0]", "Argument[-1].Element", "value", "manual"]
       - ["java.util.concurrent", "TransferQueue", True, "tryTransfer", "(Object)", "", "Argument[0]", "Argument[-1].Element", "value", "manual"]
       - ["java.util.concurrent", "TransferQueue", True, "tryTransfer", "(Object,long,TimeUnit)", "", "Argument[0]", "Argument[-1].Element", "value", "manual"]
@@ -29,3 +27,8 @@ extensions:
       extensible: neutralModel
     data:
       - ["java.util.concurrent", "CountDownLatch", "countDown", "()", "manual"]
+
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.util.concurrent", "CountDownLatch", "CountDownLatch", "(int)", "manual"] # value-numeric
+      - ["java.util.concurrent", "CountDownLatch", "getCount", "()", "manual"]          # value-numeric
diff --git a/java/ql/lib/ext/java.util.model.yml b/java/ql/lib/ext/java.util.model.yml
@@ -369,8 +369,6 @@ extensions:
       - ["java.util", "Collections", "emptyList", "()", "manual"]
       - ["java.util", "Collections", "emptyMap", "()", "manual"]
       - ["java.util", "Collections", "emptySet", "()", "manual"]
-      - ["java.util", "Date", "Date", "(long)", "manual"]
-      - ["java.util", "Date", "getTime", "()", "manual"]
       - ["java.util", "Iterator", "hasNext", "()", "manual"]
       - ["java.util", "List", "clear", "()", "manual"]
       - ["java.util", "List", "contains", "(Object)", "manual"]
@@ -390,3 +388,8 @@ extensions:
       - ["java.util", "Set", "size", "()", "manual"]
       - ["java.util", "UUID", "randomUUID", "()", "manual"]
       - ["java.util", "UUID", "toString", "()", "manual"]
+
+      # The below APIs have numeric flow and are currently being stored as neutral models.
+      # These may be changed to summary models with kinds "value-numeric" and "taint-numeric" (or similar) in the future.
+      - ["java.util", "Date", "Date", "(long)", "manual"] # taint-numeric
+      - ["java.util", "Date", "getTime", "()", "manual"]  # taint-numeric
diff --git a/java/ql/src/Telemetry/ExternalApi.qll b/java/ql/src/Telemetry/ExternalApi.qll
@@ -47,6 +47,7 @@ class ExternalApi extends Callable {
    * Gets information about the external API in the form expected by the CSV modeling framework.
    */
   string getApiName() {
+    this.getName() = "append" and
     result =
       this.getDeclaringType().getPackage() + "." + this.getDeclaringType().getSourceDeclaration() +
         "#" + this.getName() + paramsString(this)
diff --git a/java/ql/test/ext/TestModels/Test.java b/java/ql/test/ext/TestModels/Test.java
@@ -6,8 +6,6 @@
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.util.StringJoiner;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -38,20 +36,10 @@ public void test() throws Exception {
             Throwable t = new Throwable((Throwable)source());
             sink((Throwable)t.getCause()); // $hasValueFlow
 
-            Integer x = (Integer)source();
-            int y = x;
-            sink(String.valueOf(y)); // $hasTaintFlow
-
-            String s1 = (String)source();
-            sink(Integer.parseInt(s1)); // $hasTaintFlow
-
             String s2 = (String)source();
             int i = 0;
             sink(s2.charAt(i)); // $hasTaintFlow
 
-            String s3 = (String)source();
-            sink(new BigDecimal(s3)); // $hasTaintFlow
-
             ResultSet rs = (ResultSet)source();
             sink(rs.getString("")); // $hasTaintFlow
         }
@@ -76,66 +64,19 @@ public void test() throws Exception {
             sink((String)e4.getMessage());  // $hasValueFlow
             sink((Throwable)e4.getCause()); // $hasValueFlow
 
-            Integer i1 = (Integer)source();
-            sink(i1.intValue()); // $hasTaintFlow
-
-            int i2 = (int)source();
-            sink(Integer.toString(i2)); // $hasTaintFlow
-
-            int i3 = (int)source();
-            sink(Integer.valueOf(i3)); // $hasTaintFlow
-
-            Long l1 = (Long)source();
-            sink(l1.longValue()); // $hasTaintFlow
-
-            String s1 = (String)source();
-            sink(Long.parseLong(s1)); // $hasTaintFlow
-
-            Long l2 = (Long)source();
-            sink(l2.toString()); // $hasTaintFlow
-
-            long l3 = (long)source();
-            sink(String.valueOf(l3)); // $hasTaintFlow
-
             System.setProperty("testKey", (String)source());
             sink(System.getProperty("testKey")); // $hasValueFlow
 
-            // java.math
-            long l4 = (long)source();
-            sink(BigDecimal.valueOf(l4)); // $hasTaintFlow
-
-            double d1 = (double)source();
-            sink(BigDecimal.valueOf(d1)); // $hasTaintFlow
-
-            int i4 = (int)source();
-            int i5 = (int)source();
-            sink(Math.min(i4, i5)); // $hasValueFlow
-            sink(Math.min(i4, 42)); // $hasValueFlow
-            sink(Math.min(42, i5)); // $hasValueFlow
-
             // java.sql
             Connection con = DriverManager.getConnection("");
             PreparedStatement ps1 = con.prepareStatement("UPDATE EMPLOYEES SET NAME = ? WHERE ID = ?");
             ps1.setString(1, (String)source());
             sink(ps1); // $hasValueFlow
-            PreparedStatement ps2 = con.prepareStatement("UPDATE EMPLOYEES SET NAME = ? WHERE ID = ?");
-            ps2.setInt(2, (int)source());
-            sink(ps2); // $hasValueFlow
-
-            ResultSet rs = (ResultSet)source();
-            sink(rs.getInt("")); // $hasTaintFlow
 
             // java.util.concurrent.atomic
-            AtomicInteger ai = new AtomicInteger((int)source());
-            sink(ai.get());  // $hasValueFlow
-
             AtomicReference ar = new AtomicReference(source());
             sink(ar.get());  // $hasValueFlow
 
-            // java.util.concurrent
-            CountDownLatch cdl = new CountDownLatch((int)source());
-            sink(cdl.getCount()); // $hasValueFlow
-
             // java.util.function
             Function<Object, Object> func = a -> a + "";
             sink(func.apply(source()));  // $hasTaintFlow
