Automodel: Apply negative characteristics only to endpoints of the right kind.

java/ql/automodel/src/AutomodelApplicationModeCharacteristics.qll

@@ -259,7 +259,9 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig
 
   class EndpointType = AutomodelEndpointTypes::EndpointType;
 
-  class NegativeEndpointType = AutomodelEndpointTypes::NegativeSinkType;
+  class SinkType = AutomodelEndpointTypes::SinkType;
+
+  class SourceType = AutomodelEndpointTypes::SourceType;
 
   class RelatedLocation = Location::Top;
 

java/ql/automodel/src/AutomodelApplicationModeExtractNegativeExamples.ql

@@ -48,18 +48,20 @@ from
 where
   endpoint = getSampleForCharacteristic(characteristic, 100) and
   extensibleType = endpoint.getExtensibleType() and
+  // the node is know not to be an endpoint of any appropriate type
+  forall(EndpointType tp | tp = endpoint.getAPotentialType() |
+    characteristic.hasImplications(tp, false, _)
+  ) and
+  // the lowest confidence across all endpoint types should be at least highConfidence
+  confidence = min(float c | characteristic.hasImplications(endpoint.getAPotentialType(), false, c)) and
   confidence >= SharedCharacteristics::highConfidence() and
-  characteristic.hasImplications(any(NegativeSinkType negative), true, confidence) and
   meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, output, isVarargsArray) and
-  // It's valid for a node to satisfy the logic for both `isSink` and `isSanitizer`, but in that case it will be
-  // treated by the actual query as a sanitizer, since the final logic is something like
-  // `isSink(n) and not isSanitizer(n)`. We don't want to include such nodes as negative examples in the prompt, because
-  // they're ambiguous and might confuse the model, so we explicitly exclude all known sinks from the negative examples.
-  not exists(EndpointCharacteristic characteristic2, float confidence2, SinkType positiveType |
-    not positiveType instanceof NegativeSinkType and
+  // It's valid for a node to be both a potential source/sanitizer and a sink. We don't want to include such nodes
+  // as negative examples in the prompt, because they're ambiguous and might confuse the model, so we explicitly them here.
+  not exists(EndpointCharacteristic characteristic2, float confidence2, EndpointType type2 |
     characteristic2.appliesToEndpoint(endpoint) and
     confidence2 >= SharedCharacteristics::maximalConfidence() and
-    characteristic2.hasImplications(positiveType, true, confidence2)
+    characteristic2.hasImplications(type2, true, confidence2)
   ) and
   message = characteristic
 select endpoint.asNode(),

java/ql/automodel/src/AutomodelEndpointTypes.qll

@@ -30,11 +30,6 @@ abstract class SinkType extends EndpointType {
   SinkType() { any() }
 }
 
-/** The `Negative` class for non-sinks. */
-class NegativeSinkType extends SinkType {
-  NegativeSinkType() { this = "non-sink" }
-}
-
 /** A sink relevant to the SQL injection query */
 class SqlInjectionSinkType extends SinkType {
   SqlInjectionSinkType() { this = "sql-injection" }

java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll

@@ -214,7 +214,9 @@ module FrameworkCandidatesImpl implements SharedCharacteristics::CandidateSig {
 
   class EndpointType = AutomodelEndpointTypes::EndpointType;
 
-  class NegativeEndpointType = AutomodelEndpointTypes::NegativeSinkType;
+  class SinkType = AutomodelEndpointTypes::SinkType;
+
+  class SourceType = AutomodelEndpointTypes::SourceType;
 
   class RelatedLocation = Location::Top;
 

java/ql/automodel/src/AutomodelFrameworkModeExtractNegativeExamples.ql

@@ -21,18 +21,20 @@ from
 where
   endpoint.getExtensibleType() = extensibleType and
   characteristic.appliesToEndpoint(endpoint) and
+  // the node is known not to be an endpoint of any appropriate type
+  forall(EndpointType tp | tp = endpoint.getAPotentialType() |
+    characteristic.hasImplications(tp, false, _)
+  ) and
+  // the lowest confidence across all endpoint types should be at least highConfidence
+  confidence = min(float c | characteristic.hasImplications(endpoint.getAPotentialType(), false, c)) and
   confidence >= SharedCharacteristics::highConfidence() and
-  characteristic.hasImplications(any(NegativeSinkType negative), true, confidence) and
   meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, output, parameterName) and
-  // It's valid for a node to satisfy the logic for both `isSink` and `isSanitizer`, but in that case it will be
-  // treated by the actual query as a sanitizer, since the final logic is something like
-  // `isSink(n) and not isSanitizer(n)`. We don't want to include such nodes as negative examples in the prompt, because
-  // they're ambiguous and might confuse the model, so we explicitly exclude all known sinks from the negative examples.
-  not exists(EndpointCharacteristic characteristic2, float confidence2, SinkType positiveType |
-    not positiveType instanceof NegativeSinkType and
+  // It's valid for a node to be both a potential source/sanitizer and a sink. We don't want to include such nodes
+  // as negative examples in the prompt, because they're ambiguous and might confuse the model, so we explicitly them here.
+  not exists(EndpointCharacteristic characteristic2, float confidence2, EndpointType type2 |
     characteristic2.appliesToEndpoint(endpoint) and
     confidence2 >= SharedCharacteristics::maximalConfidence() and
-    characteristic2.hasImplications(positiveType, true, confidence2)
+    characteristic2.hasImplications(type2, true, confidence2)
   ) and
   message = characteristic
 select endpoint,

java/ql/automodel/src/AutomodelSharedCharacteristics.qll

@@ -33,14 +33,19 @@ signature module CandidateSig {
   class RelatedLocationType;
 
   /**
-   * A class kind for an endpoint.
+   * An endpoint type considered by this specification.
    */
   class EndpointType extends string;
 
   /**
-   * An EndpointType that denotes the absence of any sink.
+   * A sink endpoint type considered by this specification.
    */
-  class NegativeEndpointType extends EndpointType;
+  class SinkType extends EndpointType;
+
+  /**
+   * A source endpoint type considered by this specification.
+   */
+  class SourceType extends EndpointType;
 
   /**
    * Gets the endpoint as a location.
@@ -105,15 +110,14 @@ module SharedCharacteristics<CandidateSig Candidate> {
   }
 
   /**
-   * Holds if `endpoint` is modeled as `endpointType` (endpoint type must not be negative).
+   * Holds if `endpoint` is modeled as `endpointType`.
    */
   predicate isKnownAs(
     Candidate::Endpoint endpoint, Candidate::EndpointType endpointType,
     EndpointCharacteristic characteristic
   ) {
     // If the list of characteristics includes positive indicators with maximal confidence for this class, then it's a
     // known sink for the class.
-    not endpointType instanceof Candidate::NegativeEndpointType and
     characteristic.appliesToEndpoint(endpoint) and
     characteristic.hasImplications(endpointType, true, maximalConfidence())
   }
@@ -125,7 +129,6 @@ module SharedCharacteristics<CandidateSig Candidate> {
    * A candidate is an endpoint that cannot be excluded from `endpointType` based on its characteristics.
    */
   predicate isCandidate(Candidate::Endpoint endpoint, Candidate::EndpointType endpointType) {
-    not endpointType instanceof Candidate::NegativeEndpointType and
     endpointType = endpoint.getAPotentialType() and
     not exists(getAnExcludingCharacteristic(endpoint, endpointType))
   }
@@ -143,26 +146,16 @@ module SharedCharacteristics<CandidateSig Candidate> {
   }
 
   /**
-   * Gets a characteristics that disbar `endpoint` from being a candidate for `endpointType`.
+   * Gets a characteristics that disbar `endpoint` from being a candidate for `endpointType`
+   * with at least medium confidence.
    */
   EndpointCharacteristic getAnExcludingCharacteristic(
     Candidate::Endpoint endpoint, Candidate::EndpointType endpointType
   ) {
-    // An endpoint is a sink candidate if none of its characteristics give much indication whether or not it is a sink.
-    not endpointType instanceof Candidate::NegativeEndpointType and
     result.appliesToEndpoint(endpoint) and
-    (
-      // Exclude endpoints that have a characteristic that implies they're not sinks for _any_ sink type.
-      exists(float confidence |
-        confidence >= mediumConfidence() and
-        result.hasImplications(any(Candidate::NegativeEndpointType t), true, confidence)
-      )
-      or
-      // Exclude endpoints that have a characteristic that implies they're not sinks for _this particular_ sink type.
-      exists(float confidence |
-        confidence >= mediumConfidence() and
-        result.hasImplications(endpointType, false, confidence)
-      )
+    exists(float confidence |
+      confidence >= mediumConfidence() and
+      result.hasImplications(endpointType, false, confidence)
     )
   }
 
@@ -253,8 +246,8 @@ module SharedCharacteristics<CandidateSig Candidate> {
     override predicate hasImplications(
       Candidate::EndpointType endpointType, boolean isPositiveIndicator, float confidence
     ) {
-      endpointType instanceof Candidate::NegativeEndpointType and
-      isPositiveIndicator = true and
+      endpointType instanceof Candidate::SinkType and
+      isPositiveIndicator = false and
       confidence = highConfidence()
     }
   }
@@ -272,8 +265,8 @@ module SharedCharacteristics<CandidateSig Candidate> {
     override predicate hasImplications(
       Candidate::EndpointType endpointType, boolean isPositiveIndicator, float confidence
     ) {
-      endpointType instanceof Candidate::NegativeEndpointType and
-      isPositiveIndicator = true and
+      endpointType instanceof Candidate::SinkType and
+      isPositiveIndicator = false and
       confidence = mediumConfidence()
     }
   }
@@ -293,8 +286,8 @@ module SharedCharacteristics<CandidateSig Candidate> {
     override predicate hasImplications(
       Candidate::EndpointType endpointType, boolean isPositiveIndicator, float confidence
     ) {
-      endpointType instanceof Candidate::NegativeEndpointType and
-      isPositiveIndicator = true and
+      endpointType instanceof Candidate::SinkType and
+      isPositiveIndicator = false and
       confidence = mediumConfidence()
     }
   }

java/ql/automodel/test/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractCandidates.expected

@@ -7,8 +7,10 @@
 | Test.java:36:10:38:3 | newInputStream(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:36:10:38:3 | newInputStream(...) | CallContext | Test.java:36:10:38:3 | newInputStream(...) | MethodDoc | Test.java:36:10:38:3 | newInputStream(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | Test.java:37:4:37:11 | openPath | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:36:10:38:3 | newInputStream(...) | CallContext | Test.java:37:4:37:11 | openPath | MethodDoc | Test.java:37:4:37:11 | openPath | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://newInputStream:1:1:1:1 | newInputStream | name | file://(Path,OpenOption[]):1:1:1:1 | (Path,OpenOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://:1:1:1:1 |  | output | file://false:1:1:1:1 | false | isVarargsArray | file://ai-manual:1:1:1:1 | ai-manual | alreadyAiModeled | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |
 | Test.java:43:4:43:22 | get(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:43:4:43:22 | get(...) | CallContext | Test.java:43:4:43:22 | get(...) | MethodDoc | Test.java:43:4:43:22 | get(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Paths:1:1:1:1 | Paths | type | file://false:1:1:1:1 | false | subtypes | file://get:1:1:1:1 | get | name | file://(String,String[]):1:1:1:1 | (String,String[]) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
+| Test.java:48:10:50:3 | compareTo(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:48:10:50:3 | compareTo(...) | CallContext | Test.java:48:10:50:3 | compareTo(...) | MethodDoc | Test.java:48:10:50:3 | compareTo(...) | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | Test.java:54:3:59:3 | walk(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:54:3:59:3 | walk(...) | CallContext | Test.java:54:3:59:3 | walk(...) | MethodDoc | Test.java:54:3:59:3 | walk(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | Test.java:56:4:56:4 | o | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:54:3:59:3 | walk(...) | CallContext | Test.java:54:3:59:3 | walk(...) | MethodDoc | Test.java:54:3:59:3 | walk(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[1]:1:1:1:1 | Argument[1] | input | file://:1:1:1:1 |  | output | file://true:1:1:1:1 | true | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |
 | Test.java:63:3:63:3 | c | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:63:3:63:20 | getInputStream(...) | CallContext | Test.java:63:3:63:3 | c | MethodDoc | Test.java:63:3:63:3 | c | ClassDoc | file://java.net:1:1:1:1 | java.net | package | file://URLConnection:1:1:1:1 | URLConnection | type | file://true:1:1:1:1 | true | subtypes | file://getInputStream:1:1:1:1 | getInputStream | name | file://():1:1:1:1 | () | signature | file://Argument[this]:1:1:1:1 | Argument[this] | input | file://:1:1:1:1 |  | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |
 | Test.java:68:30:68:47 | writer | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:68:30:68:47 | writer | CallContext | Test.java:68:30:68:47 | writer | MethodDoc | Test.java:68:30:68:47 | writer | ClassDoc | file://java.lang:1:1:1:1 | java.lang | package | file://Throwable:1:1:1:1 | Throwable | type | file://true:1:1:1:1 | true | subtypes | file://printStackTrace:1:1:1:1 | printStackTrace | name | file://(PrintWriter):1:1:1:1 | (PrintWriter) | signature | file://:1:1:1:1 |  | input | file://Parameter[0]:1:1:1:1 | Parameter[0] | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | Test.java:86:3:88:3 | list(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:86:3:88:3 | list(...) | CallContext | Test.java:86:3:88:3 | list(...) | MethodDoc | Test.java:86:3:88:3 | list(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://list:1:1:1:1 | list | name | file://(Path):1:1:1:1 | (Path) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
+| Test.java:87:4:87:29 | createDirectories(...) | Related locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:87:4:87:29 | createDirectories(...) | CallContext | Test.java:87:4:87:29 | createDirectories(...) | MethodDoc | Test.java:87:4:87:29 | createDirectories(...) | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://createDirectories:1:1:1:1 | createDirectories | name | file://(Path,FileAttribute[]):1:1:1:1 | (Path,FileAttribute[]) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://:1:1:1:1 |  | alreadyAiModeled | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |

java/ql/automodel/test/AutomodelApplicationModeExtraction/AutomodelApplicationModeExtractNegativeExamples.expected

@@ -1,3 +1,2 @@
-| Test.java:48:10:50:3 | compareTo(...) | known sanitizer\nrelated locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:48:10:50:3 | compareTo(...) | CallContext | Test.java:48:10:50:3 | compareTo(...) | MethodDoc | Test.java:48:10:50:3 | compareTo(...) | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://:1:1:1:1 |  | input | file://ReturnValue:1:1:1:1 | ReturnValue | output | file://false:1:1:1:1 | false | isVarargsArray | file://sourceModel:1:1:1:1 | sourceModel | extensibleType |
 | Test.java:49:4:49:5 | f2 | known non-sink\nrelated locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:48:10:50:3 | compareTo(...) | CallContext | Test.java:49:4:49:5 | f2 | MethodDoc | Test.java:49:4:49:5 | f2 | ClassDoc | file://java.io:1:1:1:1 | java.io | package | file://File:1:1:1:1 | File | type | file://true:1:1:1:1 | true | subtypes | file://compareTo:1:1:1:1 | compareTo | name | file://(File):1:1:1:1 | (File) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://:1:1:1:1 |  | output | file://false:1:1:1:1 | false | isVarargsArray | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |
 | Test.java:55:4:55:4 | p | taint step\nrelated locations: $@, $@, $@.\nmetadata: $@, $@, $@, $@, $@, $@, $@, $@, $@. | Test.java:54:3:59:3 | walk(...) | CallContext | Test.java:55:4:55:4 | p | MethodDoc | Test.java:55:4:55:4 | p | ClassDoc | file://java.nio.file:1:1:1:1 | java.nio.file | package | file://Files:1:1:1:1 | Files | type | file://false:1:1:1:1 | false | subtypes | file://walk:1:1:1:1 | walk | name | file://(Path,FileVisitOption[]):1:1:1:1 | (Path,FileVisitOption[]) | signature | file://Argument[0]:1:1:1:1 | Argument[0] | input | file://:1:1:1:1 |  | output | file://false:1:1:1:1 | false | isVarargsArray | file://sinkModel:1:1:1:1 | sinkModel | extensibleType |