Java: only assume that _manual_ MaD sinks have been fully modeled

Author: Stephan Brandauer

java/ql/src/Telemetry/AutomodelApplicationModeCharacteristics.qll

@@ -67,13 +67,13 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig
 
   predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2;
 
-  predicate isSink(Endpoint e, string kind) {
+  predicate isSink(Endpoint e, string kind, string provenance) {
     exists(string package, string type, string name, string signature, string ext, string input |
       sinkSpec(e, package, type, name, signature, ext, input) and
-      ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, _)
+      ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
     )
     or
-    isCustomSink(e, kind)
+    isCustomSink(e, kind) and provenance = "custom-sink"
   }
 
   predicate isNeutral(Endpoint e) {
@@ -200,7 +200,7 @@ private class UnexploitableIsCharacteristic extends CharacteristicsImpl::NotASin
   UnexploitableIsCharacteristic() { this = "unexploitable (is-style boolean method)" }
 
   override predicate appliesToEndpoint(Endpoint e) {
-    not ApplicationCandidatesImpl::isSink(e, _) and
+    not ApplicationCandidatesImpl::isSink(e, _, _) and
     ApplicationModeGetCallable::getCallable(e).getName().matches("is%") and
     ApplicationModeGetCallable::getCallable(e).getReturnType() instanceof BooleanType
   }
@@ -218,7 +218,7 @@ private class UnexploitableExistsCharacteristic extends CharacteristicsImpl::Not
   UnexploitableExistsCharacteristic() { this = "unexploitable (existence-checking boolean method)" }
 
   override predicate appliesToEndpoint(Endpoint e) {
-    not ApplicationCandidatesImpl::isSink(e, _) and
+    not ApplicationCandidatesImpl::isSink(e, _, _) and
     exists(Callable callable |
       callable = ApplicationModeGetCallable::getCallable(e) and
       callable.getName().toLowerCase() = ["exists", "notexists"] and
@@ -313,7 +313,8 @@ private class NonPublicMethodCharacteristic extends CharacteristicsImpl::Uninter
 
 /**
  * A negative characteristic that indicates that an endpoint is a non-sink argument to a method whose sinks have already
- * been modeled.
+ * been modeled _manually_. This is restricted to manual sinks only, because only during the manual process do we have
+ * the expectation that all sinks present in a method have been considered.
  *
  * WARNING: These endpoints should not be used as negative samples for training, because some sinks may have been missed
  * when the method was modeled. Specifically, as we start using ATM to merge in new declarations, we can be less sure
@@ -324,13 +325,13 @@ private class NonPublicMethodCharacteristic extends CharacteristicsImpl::Uninter
 private class OtherArgumentToModeledMethodCharacteristic extends CharacteristicsImpl::LikelyNotASinkCharacteristic
 {
   OtherArgumentToModeledMethodCharacteristic() {
-    this = "other argument to a method that has already been modeled"
+    this = "other argument to a method that has already been modeled manually"
   }
 
   override predicate appliesToEndpoint(Endpoint e) {
-    not ApplicationCandidatesImpl::isSink(e, _) and
+    not ApplicationCandidatesImpl::isSink(e, _, _) and
     exists(DataFlow::Node otherSink, Call c |
-      ApplicationCandidatesImpl::isSink(otherSink, _) and
+      ApplicationCandidatesImpl::isSink(otherSink, _, "manual") and
       c = otherSink.asExpr().(Argument).getCall() and
       e.asExpr() in [c.getQualifier(), c.getAnArgument()] and
       e != otherSink

java/ql/src/Telemetry/AutomodelApplicationModeExtractCandidates.ql

@@ -64,7 +64,7 @@ where
   // label it as a sink for one of the sink types of query B, for which it's already a known sink. This would result in
   // overlap between our detected sinks and the pre-existing modeling. We assume that, if a sink has already been
   // modeled in a MaD model, then it doesn't belong to any additional sink types, and we don't need to reexamine it.
-  not CharacteristicsImpl::isSink(endpoint, _) and
+  not CharacteristicsImpl::isSink(endpoint, _, _) and
   meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input) and
   // The message is the concatenation of all sink types for which this endpoint is known neither to be a sink nor to be
   // a non-sink, and we surface only endpoints that have at least one such sink type.

java/ql/src/Telemetry/AutomodelFrameworkModeCharacteristics.qll

@@ -50,10 +50,10 @@ module FrameworkCandidatesImpl implements SharedCharacteristics::CandidateSig {
 
   predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2;
 
-  predicate isSink(Endpoint e, string kind) {
+  predicate isSink(Endpoint e, string kind, string provenance) {
     exists(string package, string type, string name, string signature, string ext, string input |
       sinkSpec(e, package, type, name, signature, ext, input) and
-      ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, _)
+      ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
     )
   }
 
@@ -154,7 +154,7 @@ private class UnexploitableIsCharacteristic extends CharacteristicsImpl::NotASin
   UnexploitableIsCharacteristic() { this = "unexploitable (is-style boolean method)" }
 
   override predicate appliesToEndpoint(Endpoint e) {
-    not FrameworkCandidatesImpl::isSink(e, _) and
+    not FrameworkCandidatesImpl::isSink(e, _, _) and
     FrameworkModeGetCallable::getCallable(e).getName().matches("is%") and
     FrameworkModeGetCallable::getCallable(e).getReturnType() instanceof BooleanType
   }
@@ -172,7 +172,7 @@ private class UnexploitableExistsCharacteristic extends CharacteristicsImpl::Not
   UnexploitableExistsCharacteristic() { this = "unexploitable (existence-checking boolean method)" }
 
   override predicate appliesToEndpoint(Endpoint e) {
-    not FrameworkCandidatesImpl::isSink(e, _) and
+    not FrameworkCandidatesImpl::isSink(e, _, _) and
     exists(Callable callable |
       callable = FrameworkModeGetCallable::getCallable(e) and
       callable.getName().toLowerCase() = ["exists", "notexists"] and

java/ql/src/Telemetry/AutomodelFrameworkModeExtractCandidates.ql

@@ -28,7 +28,7 @@ where
   // label it as a sink for one of the sink types of query B, for which it's already a known sink. This would result in
   // overlap between our detected sinks and the pre-existing modeling. We assume that, if a sink has already been
   // modeled in a MaD model, then it doesn't belong to any additional sink types, and we don't need to reexamine it.
-  not CharacteristicsImpl::isSink(endpoint, _) and
+  not CharacteristicsImpl::isSink(endpoint, _, _) and
   meta.hasMetadata(endpoint, package, type, subtypes, name, signature, input, parameterName) and
   // The message is the concatenation of all sink types for which this endpoint is known neither to be a sink nor to be
   // a non-sink, and we surface only endpoints that have at least one such sink type.

java/ql/src/Telemetry/AutomodelSharedCharacteristics.qll

@@ -58,9 +58,9 @@ signature module CandidateSig {
   predicate isSanitizer(Endpoint e, EndpointType t);
 
   /**
-   * Holds if `e` is a sink with the label `kind`.
+   * Holds if `e` is a sink with the label `kind`, and provenance `provenance`.
    */
-  predicate isSink(Endpoint e, string kind);
+  predicate isSink(Endpoint e, string kind, string provenance);
 
   /**
    * Holds if `e` is not a sink of any kind.
@@ -87,7 +87,7 @@ signature module CandidateSig {
  *   implementations of endpoint characteristics exported by this module.
  */
 module SharedCharacteristics<CandidateSig Candidate> {
-  predicate isSink = Candidate::isSink/2;
+  predicate isSink = Candidate::isSink/3;
 
   predicate isNeutral = Candidate::isNeutral/1;
 
@@ -282,7 +282,9 @@ module SharedCharacteristics<CandidateSig Candidate> {
         this = madKind + "-characteristic"
       }
 
-      override predicate appliesToEndpoint(Candidate::Endpoint e) { Candidate::isSink(e, madKind) }
+      override predicate appliesToEndpoint(Candidate::Endpoint e) {
+        Candidate::isSink(e, madKind, _)
+      }
 
       override Candidate::EndpointType getSinkType() { result = endpointType }
     }