From afd9f720f0a2106b456b6dfdc023e6b147823a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pole=C5=A1ovsk=C3=BD?= Date: Wed, 16 Aug 2017 16:48:22 +0200 Subject: [PATCH 1/3] Fix definitions of File.createTempFile #328 Copy & Paste issue probably, stack doesn't contain "this" object (and "new" copy as in ) --- plugin/src/main/resources/taint-config/java-net.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/resources/taint-config/java-net.txt b/plugin/src/main/resources/taint-config/java-net.txt index d0513d58c..7a4d72120 100644 --- 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 From 7461999eaa500628c8e9b049c7a29728d3f4064c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pole=C5=A1ovsk=C3=BD?= Date: Wed, 16 Aug 2017 17:07:48 +0200 Subject: [PATCH 2/3] Include the failing method config in assert #328 Also show full configuration line with method and type signature so that it's easier to find and fix in defintion files --- .../findsecbugs/taintanalysis/TaintConfig.java | 3 +++ .../taintanalysis/TaintFrameModelingVisitor.java | 2 +- .../findsecbugs/taintanalysis/TaintMethodConfig.java | 11 ++++++++++- 3 files changed, 14 insertions(+), 2 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 index 2e2f4065d..8a06c4774 100644 --- 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 index 277696de4..467141f70 100644 --- 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 index 2413e3e3e..3d98befa5 100644 --- 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 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; + } } From 5fd62d9f653318a7db69ead17de8d6c72a09fdfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pole=C5=A1ovsk=C3=BD?= Date: Wed, 16 Aug 2017 17:46:15 +0200 Subject: [PATCH 3/3] File#createTempFile is also sink for Path Traversal --- .../src/main/resources/injection-sinks/path-traversal-in.txt | 3 +++ .../h3xstream/findsecbugs/file/PathTraversalDetectorTest.java | 4 ++-- .../src/test/java/testcode/pathtraversal/PathTraversal.java | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/plugin/src/main/resources/injection-sinks/path-traversal-in.txt b/plugin/src/main/resources/injection-sinks/path-traversal-in.txt index 5b9db2f79..00cc8b544 100644 --- 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.(Ljava/lang/String;)V:0 java/io/FileInputStream.(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 \ No newline at end of file diff --git a/plugin/src/test/java/com/h3xstream/findsecbugs/file/PathTraversalDetectorTest.java b/plugin/src/test/java/com/h3xstream/findsecbugs/file/PathTraversalDetectorTest.java index 211339489..770cb3838 100644 --- 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 index ac411015b..e61c084d4 100644 --- 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")); } }