From ae23920a6d5685587bd16e6db64d942a44af9ad7 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max-schaefer@github.com>
Date: Wed, 17 Jan 2024 15:36:31 +0000
Subject: [PATCH] Fix spurious source models for primitive types in framework
 mode.

---
 .../src/AutomodelFrameworkModeCharacteristics.qll        | 9 +++------
 java/ql/automodel/src/AutomodelJavaUtil.qll              | 9 +++++++++
 .../AutomodelFrameworkModeExtraction/java/io/File.java   | 2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll b/java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll
index a600e7306b9..6981e0369b5 100644
--- a/java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll
+++ b/java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll
@@ -38,18 +38,15 @@ newtype TFrameworkModeEndpoint =
   } or
   TOverridableParameter(Method m, Parameter p) {
     AutomodelJavaUtil::isFromSource(p) and
+    not AutomodelJavaUtil::isUnexploitableType(p.getType()) and
     p.getCallable() = m and
     m instanceof ModelExclusions::ModelApi and
-    not m.getDeclaringType().isFinal() and
-    not m.isFinal() and
-    not m.isStatic()
+    AutomodelJavaUtil::isOverridable(m)
   } or
   TOverridableQualifier(Method m) {
     AutomodelJavaUtil::isFromSource(m) and
     m instanceof ModelExclusions::ModelApi and
-    not m.getDeclaringType().isFinal() and
-    not m.isFinal() and
-    not m.isStatic()
+    AutomodelJavaUtil::isOverridable(m)
   }
 
 /**
diff --git a/java/ql/automodel/src/AutomodelJavaUtil.qll b/java/ql/automodel/src/AutomodelJavaUtil.qll
index 1504d14df62..a57edf92150 100644
--- a/java/ql/automodel/src/AutomodelJavaUtil.qll
+++ b/java/ql/automodel/src/AutomodelJavaUtil.qll
@@ -111,3 +111,12 @@ predicate isUnexploitableType(Type tp) {
   tp instanceof NumberType or
   tp instanceof VoidType
 }
+
+/**
+ * Holds if the given method can be overridden, that is, it is not final,
+ * static, or private.
+ */
+predicate isOverridable(Method m) {
+  not m.getDeclaringType().isFinal() and not m.isFinal() and
+  not m.isStatic() and not m.isPrivate()
+}
\ No newline at end of file
diff --git a/java/ql/automodel/test/AutomodelFrameworkModeExtraction/java/io/File.java b/java/ql/automodel/test/AutomodelFrameworkModeExtraction/java/io/File.java
index 2ced57ffb1f..1554a2eca7f 100644
--- a/java/ql/automodel/test/AutomodelFrameworkModeExtraction/java/io/File.java
+++ b/java/ql/automodel/test/AutomodelFrameworkModeExtraction/java/io/File.java
@@ -7,7 +7,7 @@ public class File {
         return 0;
     }
 
-    public boolean setLastModified(long time) { // $ sinkModel=setLastModified(long):Argument[this] sourceModel=setLastModified(long):Parameter[this] SPURIOUS: sourceModel=setLastModified(long):Parameter[0] // time is not a candidate (primitive type)
+    public boolean setLastModified(long time) { // $ sinkModel=setLastModified(long):Argument[this] sourceModel=setLastModified(long):Parameter[this] // time is not a candidate (primitive type)
         return false;
     } // return value is not a source candidate because it's a primitive
 }