Merge pull request #330 from topolik/328

Fix java/io/File#createTempFile #328
find-sec-bugs · Aug 17, 2017 · 2e669c6 · 2e669c6
2 parents af59f67 + 5fd62d9
commit 2e669c6
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 6 deletions.
diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintConfig.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintConfig.java
@@ -83,6 +83,7 @@ public void receiveTaintConfig(String typeSignature, String config) throws IOExc
                         throw new IllegalStateException("Config for " + typeSignature + " already loaded");
                     }
                     TaintMethodConfig taintMethodConfig = new TaintMethodConfig(true).load(config);
+                    taintMethodConfig.setTypeSignature(typeSignature);
                     put(typeSignature, taintMethodConfig);
                     return;
                 }
@@ -104,6 +105,8 @@ public void receiveTaintConfig(String typeSignature, String config) throws IOExc
                     TaintMethodConfigWithArgumentsAndLocation methodConfig =
                             new TaintMethodConfigWithArgumentsAndLocation().load(config);
 
+                    methodConfig.setTypeSignature(typeSignature);
+
                     String key = typeSignature + '@' + methodConfig.getLocation();
                     taintMethodConfigWithArgumentsAndLocationMap.put(key, methodConfig);
                     return;

diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java
@@ -586,7 +586,7 @@ private void transferTaintToMutables(TaintMethodConfig methodConfig, Taint taint
                 if (mutableStackIndex >= stackDepth) {
                     if (!Constants.CONSTRUCTOR_NAME.equals(methodDescriptor.getName())
                             && !Constants.STATIC_INITIALIZER_NAME.equals(methodDescriptor.getName())) {
-                        assert false : "Out of bounds mutables in " + methodDescriptor;
+                        assert false : "Out of bounds mutables in " + methodDescriptor + " Method Config: " + methodConfig.toString();
                     }
                     continue; // ignore if assertions disabled or if in constructor
                 }

diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodConfig.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodConfig.java
@@ -36,6 +36,7 @@ public class TaintMethodConfig implements TaintTypeConfig {
     private Taint outputTaint = null;
     private final Set<Integer> mutableStackIndices;
     private final boolean isConfigured;
+    private String typeSignature;
     public static final TaintMethodConfig SAFE_CONFIG;
     protected static final Pattern fullMethodPattern;
     protected static final Pattern configPattern;
@@ -215,9 +216,13 @@ public boolean isConfigured() {
     @Override
     public String toString() {
         if (outputTaint == null) {
-            return "";
+            return typeSignature != null ? typeSignature : "";
         }
         StringBuilder sb = new StringBuilder();
+        if (typeSignature != null) {
+            sb.append(typeSignature);
+            sb.append(":");
+        }
         if (outputTaint.isUnknown() && outputTaint.hasParameters()) {
             appendJoined(sb, outputTaint.getParameters());
             Taint.State nonParametricState = outputTaint.getNonParametricState();
@@ -448,4 +453,8 @@ private boolean isTaintStateValue(String value) {
         }
         return false;
     }
+
+    public void setTypeSignature(String typeSignature) {
+        this.typeSignature = typeSignature;
+    }
 }
diff --git a/plugin/src/main/resources/injection-sinks/path-traversal-in.txt b/plugin/src/main/resources/injection-sinks/path-traversal-in.txt
@@ -8,3 +8,6 @@ java/io/FileReader.<init>(Ljava/lang/String;)V:0
 java/io/FileInputStream.<init>(Ljava/lang/String;)V:0
 
 java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;:0,1
+
+java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;)Ljava/io/File;:0,1
+java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;Ljava/io/File;)Ljava/io/File;:0,1,2
diff --git a/plugin/src/main/resources/taint-config/java-net.txt b/plugin/src/main/resources/taint-config/java-net.txt
@@ -49,6 +49,6 @@ java/net/URL.toExternalForm()Ljava/lang/String;:0
 java/net/URL.toString()Ljava/lang/String;:0
 java/net/URL.toURI()Ljava/net/URI;:0
 
-java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;)Ljava/io/File;:0,1#2,3
-java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;Ljava/io/File;)Ljava/io/File;:0,1,2#3,4
+java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;)Ljava/io/File;:0,1
+java/io/File.createTempFile(Ljava/lang/String;Ljava/lang/String;Ljava/io/File;)Ljava/io/File;:0,1,2
 java/io/File.getCanonicalPath()Ljava/lang/String;:0
diff --git a/plugin/src/test/java/com/h3xstream/findsecbugs/file/PathTraversalDetectorTest.java b/plugin/src/test/java/com/h3xstream/findsecbugs/file/PathTraversalDetectorTest.java
@@ -43,7 +43,7 @@ public void detectPathTraversal() throws Exception {
         EasyBugReporter reporter = spy(new SecurityReporter());
         analyze(files, reporter);
 
-        for (Integer line : Arrays.asList(17, 18, 19, 20, 22, 23)) {
+        for (Integer line : Arrays.asList(17, 18, 19, 20, 22, 23, 35, 36, 37)) {
             verify(reporter).doReportBug(
                     bugDefinition()
                             .bugType("PATH_TRAVERSAL_IN")
@@ -61,7 +61,7 @@ public void detectPathTraversal() throws Exception {
             );
         }
 
-        verify(reporter, times(6)).doReportBug(bugDefinition().bugType("PATH_TRAVERSAL_IN").build());
+        verify(reporter, times(9)).doReportBug(bugDefinition().bugType("PATH_TRAVERSAL_IN").build());
         verify(reporter, times(4)).doReportBug(bugDefinition().bugType("PATH_TRAVERSAL_OUT").build());
     }
 }
diff --git a/plugin/src/test/java/testcode/pathtraversal/PathTraversal.java b/plugin/src/test/java/testcode/pathtraversal/PathTraversal.java
@@ -31,5 +31,9 @@ public static void main(String[] args) throws IOException, URISyntaxException {
         new RandomAccessFile("safe", args[0]);
         new FileWriter("safe".toUpperCase());
         new File(new URI("safe"));
+
+        File.createTempFile(input, "safe");
+        File.createTempFile("safe", input);
+        File.createTempFile("safe", input, new File("safeDir"));
     }
 }