Merge pull request #12249 from jcogs33/jcogs33/add-heuristic-neutral-models

Java: add some neutral models discovered with heuristics

java/ql/lib/ext/java.io.model.yml

@@ -100,6 +100,7 @@ extensions:
       pack: codeql/java-all
       extensible: neutralModel
     data:
+      # summary neutrals
       - ["java.io", "Closeable", "close", "()", "summary", "manual"]
       - ["java.io", "DataOutput", "writeBoolean", "(boolean)", "summary", "manual"]
       - ["java.io", "File", "delete", "()", "summary", "manual"]
@@ -117,3 +118,7 @@ extensions:
       - ["java.io", "DataInput", "readLong", "()", "summary", "manual"]        # taint-numeric
       - ["java.io", "DataOutput", "writeInt", "(int)", "summary", "manual"]    # taint-numeric
       - ["java.io", "DataOutput", "writeLong", "(long)", "summary", "manual"]  # taint-numeric
+
+      # sink neutrals
+      - ["java.io", "File", "compareTo", "", "sink", "hq-manual"]
+      - ["java.io", "File", "exists", "()", "sink", "hq-manual"]

java/ql/lib/ext/java.nio.file.model.yml

@@ -81,4 +81,22 @@ extensions:
       pack: codeql/java-all
       extensible: neutralModel
     data:
+      # summary neutrals
       - ["java.nio.file", "Files", "exists", "(Path,LinkOption[])", "summary", "manual"]
+
+      # sink neutrals
+      - ["java.nio.file", "Files", "exists", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "getLastModifiedTime", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "getOwner", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "getPosixFilePermissions", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isDirectory", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isExecutable", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isHidden", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isReadable", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isRegularFile", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isSameFile", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isSymbolicLink", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "isWritable", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "notExists", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "setLastModifiedTime", "", "sink", "hq-manual"]
+      - ["java.nio.file", "Files", "size", "", "sink", "hq-manual"]

java/ql/lib/ext/java.nio.file.spi.model.yml

@@ -0,0 +1,8 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      # sink neutrals
+      - ["java.nio.file.spi", "FileSystemProvider", "isHidden", "", "sink", "hq-manual"]
+      - ["java.nio.file.spi", "FileSystemProvider", "isSameFile", "", "sink", "hq-manual"]

java/ql/lib/ext/java.text.model.yml

@@ -3,8 +3,14 @@ extensions:
       pack: codeql/java-all
       extensible: neutralModel
     data:
+      # summary neutrals
       # 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.text", "DateFormat", "format", "(Date)", "summary", "manual"]                   # taint-numeric
       - ["java.text", "DateFormat", "parse", "(String)", "summary", "manual"]                  # taint-numeric
       - ["java.text", "SimpleDateFormat", "SimpleDateFormat", "(String)", "summary", "manual"] # taint-numeric
+
+      # sink neutrals
+      - ["java.text", "Collator", "compare", "", "sink", "hq-manual"]
+      - ["java.text", "Collator", "equals", "", "sink", "hq-manual"]
+      - ["java.text", "RuleBasedCollator", "compare", "", "sink", "hq-manual"]

java/ql/lib/ext/java.util.prefs.model.yml

@@ -0,0 +1,8 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      # sink neutrals
+      - ["java.util.prefs", "AbstractPreferences", "nodeExists", "", "sink", "hq-manual"]
+      - ["java.util.prefs", "Preferences", "nodeExists", "", "sink", "hq-manual"]

java/ql/lib/ext/org.apache.hc.client5.http.protocol.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      # sink neutrals
+      - ["org.apache.hc.client5.http.protocol", "RedirectLocations", "contains", "", "sink", "hq-manual"]

java/ql/test/library-tests/neutrals/neutralsinks/NeutralSinksTest.ql

@@ -0,0 +1,40 @@
+import java
+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.ExternalFlow
+import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
+
+class SinkTest extends InlineExpectationsTest {
+  SinkTest() { this = "SinkTest" }
+
+  override string getARelevantTag() { result = "isSink" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "isSink" and
+    exists(DataFlow::Node sink |
+      sinkNode(sink, _) and
+      sink.getLocation() = location and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}
+
+class NeutralSinkTest extends InlineExpectationsTest {
+  NeutralSinkTest() { this = "NeutralSinkTest" }
+
+  override string getARelevantTag() { result = "isNeutralSink" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "isNeutralSink" and
+    exists(Call call, Callable callable |
+      call.getCallee() = callable and
+      neutralModel(callable.getDeclaringType().getCompilationUnit().getPackage().getName(),
+        callable.getDeclaringType().getSourceDeclaration().nestedName(), callable.getName(),
+        [paramsString(callable), ""], "sink", _) and
+      call.getLocation() = location and
+      element = call.toString() and
+      value = ""
+    )
+  }
+}

java/ql/test/library-tests/neutrals/neutralsinks/Test.java

@@ -0,0 +1,61 @@
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.spi.FileSystemProvider;
+import java.nio.file.LinkOption;
+import java.text.Collator;
+import java.text.RuleBasedCollator;
+import java.util.prefs.AbstractPreferences;
+import java.util.prefs.Preferences;
+import org.apache.hc.client5.http.protocol.RedirectLocations;
+
+public class Test {
+
+    public void test() throws Exception {
+
+        // java.io
+        File file = null;
+        file.exists(); // $ isNeutralSink
+        file.compareTo(null); // $ isNeutralSink
+
+        // java.nio.file
+        Files.exists(null, (LinkOption[])null); // $ isNeutralSink
+        Files.getLastModifiedTime(null, (LinkOption[])null); // $ isNeutralSink
+        Files.getOwner(null, (LinkOption[])null); // $ isNeutralSink
+        Files.getPosixFilePermissions(null, (LinkOption[])null); // $ isNeutralSink
+        Files.isDirectory(null, (LinkOption[])null); // $ isNeutralSink
+        Files.isExecutable(null); // $ isNeutralSink
+        Files.isHidden(null); // $ isNeutralSink
+        Files.isReadable(null); // $ isNeutralSink
+        Files.isRegularFile(null, (LinkOption[])null); // $ isNeutralSink
+        Files.isSameFile(null, null); // $ isNeutralSink
+        Files.isSymbolicLink(null); // $ isNeutralSink
+        Files.isWritable(null); // $ isNeutralSink
+        Files.notExists(null, (LinkOption[])null); // $ isNeutralSink
+        Files.setLastModifiedTime(null, null); // $ isNeutralSink
+        Files.size(null); // $ isNeutralSink
+
+        // java.nio.file.spi
+        FileSystemProvider fsp = null;
+        fsp.isHidden(null); // $ isNeutralSink
+        fsp.isSameFile(null, null); // $ isNeutralSink
+
+        // java.text
+        Collator c = null;
+        c.compare(null, null); // $ isNeutralSink
+        c.equals(null); // $ isNeutralSink
+        c.equals(null, null); // $ isNeutralSink
+        RuleBasedCollator rbc = null;
+        rbc.compare(null, null); // $ isNeutralSink
+
+        // java.util.prefs
+        AbstractPreferences ap = null;
+        ap.nodeExists(null); // $ isNeutralSink
+        Preferences p = null;
+        p.nodeExists(null); // $ isNeutralSink
+
+        // org.apache.hc.client5.http.protocol
+        RedirectLocations rl = null;
+        rl.contains(null); // $ isNeutralSink
+    }
+
+}

java/ql/test/library-tests/neutrals/neutralsinks/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -source 11 -target 11 -cp ${testdir}/../../../stubs/apache-http-5

java/ql/test/stubs/apache-http-5/org/apache/hc/client5/http/protocol/RedirectLocations.java

@@ -0,0 +1,111 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+ package org.apache.hc.client5.http.protocol;
+
+ import java.net.URI;
+ import java.util.ArrayList;
+ import java.util.HashSet;
+ import java.util.List;
+ import java.util.Set;
+
+ /**
+  * This class represents a collection of {@link java.net.URI}s used
+  * as redirect locations.
+  *
+  * @since 4.0
+  */
+ public final class RedirectLocations {
+
+     private final Set<URI> unique;
+     private final List<URI> all;
+
+     public RedirectLocations() {
+         super();
+         this.unique = new HashSet<>();
+         this.all = new ArrayList<>();
+     }
+
+     /**
+      * Test if the URI is present in the collection.
+      */
+     public boolean contains(final URI uri) {
+         return this.unique.contains(uri);
+     }
+
+     /**
+      * Adds a new URI to the collection.
+      */
+     public void add(final URI uri) {
+         this.unique.add(uri);
+         this.all.add(uri);
+     }
+
+     /**
+      * Returns all redirect {@link URI}s in the order they were added to the collection.
+      *
+      * @return list of all URIs
+      *
+      * @since 4.1
+      */
+     public List<URI> getAll() {
+         return new ArrayList<>(this.all);
+     }
+
+     /**
+      * Returns the URI at the specified position in this list.
+      *
+      * @param index
+      *            index of the location to return
+      * @return the URI at the specified position in this list
+      * @throws IndexOutOfBoundsException
+      *             if the index is out of range (
+      *             {@code index &lt; 0 || index &gt;= size()})
+      * @since 4.3
+      */
+     public URI get(final int index) {
+         return this.all.get(index);
+     }
+
+     /**
+      * Returns the number of elements in this list. If this list contains more
+      * than {@code Integer.MAX_VALUE} elements, returns
+      * {@code Integer.MAX_VALUE}.
+      *
+      * @return the number of elements in this list
+      * @since 4.3
+      */
+     public int size() {
+         return this.all.size();
+     }
+
+     public void clear() {
+         unique.clear();
+         all.clear();
+     }
+
+ }