From 8ee1f8ae6992cd004cbb9bc0a2fabc39ae14b08e Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 22 Sep 2023 13:33:45 +0200 Subject: [PATCH] Java: Add missing flow step for ThreadLocal.initialValue. --- .../semmle/code/java/dataflow/FlowSteps.qll | 1 + .../code/java/frameworks/ThreadLocal.qll | 40 +++++++++++++++++++ .../CWE-611/DocumentBuilderTests.java | 2 +- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 java/ql/lib/semmle/code/java/frameworks/ThreadLocal.qll diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll index b969400c0c5..2021dcc2bef 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll @@ -23,6 +23,7 @@ private module Frameworks { private import semmle.code.java.frameworks.InputStream private import semmle.code.java.frameworks.Properties private import semmle.code.java.frameworks.Protobuf + private import semmle.code.java.frameworks.ThreadLocal private import semmle.code.java.frameworks.ratpack.RatpackExec private import semmle.code.java.frameworks.stapler.Stapler } diff --git a/java/ql/lib/semmle/code/java/frameworks/ThreadLocal.qll b/java/ql/lib/semmle/code/java/frameworks/ThreadLocal.qll new file mode 100644 index 00000000000..4daf9eb84a6 --- /dev/null +++ b/java/ql/lib/semmle/code/java/frameworks/ThreadLocal.qll @@ -0,0 +1,40 @@ +import java +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSteps + +/** + * Holds if `cie` construct a `ThreadLocal` object with an overridden + * `initialValue` method with a return value of `init`, such that `init` is the + * initial value of the `ThreadLocal` object. + */ +private predicate threadLocalInitialValue(ClassInstanceExpr cie, Method initialValue, Expr init) { + exists(RefType t, ReturnStmt ret | + cie.getConstructedType().getSourceDeclaration() = t and + t.getASourceSupertype+().hasQualifiedName("java.lang", "ThreadLocal") and + ret.getResult() = init and + ret.getEnclosingCallable() = initialValue and + initialValue.hasName("initialValue") and + initialValue.getDeclaringType() = t + ) +} + +private class ThreadLocalInitialValueStore extends AdditionalStoreStep { + override predicate step(DataFlow::Node node1, DataFlow::Content c, DataFlow::Node node2) { + exists(Method initialValue, Expr init | + threadLocalInitialValue(_, initialValue, init) and + node1.asExpr() = init and + node2.(DataFlow::InstanceParameterNode).getCallable() = initialValue and + c.(DataFlow::SyntheticFieldContent).getField() = "java.lang.ThreadLocal.value" + ) + } +} + +private class ThreadLocalInitialValueStep extends AdditionalValueStep { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + exists(ClassInstanceExpr cie, Method initialValue | + threadLocalInitialValue(cie, initialValue, _) and + node1.(DataFlow::InstanceParameterNode).getCallable() = initialValue and + node2.asExpr() = cie + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-611/DocumentBuilderTests.java b/java/ql/test/query-tests/security/CWE-611/DocumentBuilderTests.java index 5761bb53136..98d95686301 100644 --- a/java/ql/test/query-tests/security/CWE-611/DocumentBuilderTests.java +++ b/java/ql/test/query-tests/security/CWE-611/DocumentBuilderTests.java @@ -129,7 +129,7 @@ class DocumentBuilderTests { public void disableExternalEntities2(Socket sock) throws Exception { DocumentBuilder builder = XML_DOCUMENT_BUILDER.get(); - builder.parse(sock.getInputStream()); // $ SPURIOUS: hasTaintFlow + builder.parse(sock.getInputStream()); // safe } }