From 797721cd316fca7015fc7e918e38d49702634ac2 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Fri, 24 Apr 2020 18:56:38 +0200 Subject: [PATCH 1/2] Test --- .../library-tests/UnsafeDeserialization/Test.java | 12 ++++++++++++ .../unsafeDeserialization.expected | 1 + .../UnsafeDeserialization/unsafeDeserialization.ql | 6 ++++++ 3 files changed, 19 insertions(+) create mode 100644 java/ql/test/library-tests/UnsafeDeserialization/Test.java create mode 100644 java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.expected create mode 100644 java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.ql diff --git a/java/ql/test/library-tests/UnsafeDeserialization/Test.java b/java/ql/test/library-tests/UnsafeDeserialization/Test.java new file mode 100644 index 00000000000..bbc59f956cd --- /dev/null +++ b/java/ql/test/library-tests/UnsafeDeserialization/Test.java @@ -0,0 +1,12 @@ +import java.io.IOException; +import java.io.ObjectInputStream; +import org.apache.commons.io.serialization.ValidatingObjectInputStream; + +class Test { + public void test() throws IOException, ClassNotFoundException { + ObjectInputStream objectStream = new ObjectInputStream(null); + ObjectInputStream validating = new ValidatingObjectInputStream(null); + objectStream.readObject(); + validating.readObject(); + } +} diff --git a/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.expected b/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.expected new file mode 100644 index 00000000000..3b02e3ebe1a --- /dev/null +++ b/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.expected @@ -0,0 +1 @@ +| Test.java:9:3:9:27 | readObject(...) | ObjectInputStream | diff --git a/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.ql b/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.ql new file mode 100644 index 00000000000..9433eba7f7f --- /dev/null +++ b/java/ql/test/library-tests/UnsafeDeserialization/unsafeDeserialization.ql @@ -0,0 +1,6 @@ +import default +import semmle.code.java.security.UnsafeDeserialization + +from Method m, MethodAccess ma +where ma.getMethod() = m and unsafeDeserialization(ma, _) +select ma, m.getDeclaringType().getName() From 39e652b26b81c2e57c2ab372574289b32c1baa5f Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 25 Mar 2020 16:17:31 +0100 Subject: [PATCH 2/2] Java: teach UnsafeDeserialization about ValidatingObjectInputStream The class org.apache.commons.io.serialization.ValidatingObjectInputStream is an implementation of ObjectInputStream that validates the deserialized classes against a white list. Therefore, this class should not be considered an unsafe deserialization sink. --- .../semmle/code/java/security/UnsafeDeserialization.qll | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll b/java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll index 042d9b436fa..555d65d2257 100644 --- a/java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll +++ b/java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll @@ -51,7 +51,14 @@ class SafeKryo extends DataFlow2::Configuration { predicate unsafeDeserialization(MethodAccess ma, Expr sink) { exists(Method m | m = ma.getMethod() | m instanceof ObjectInputStreamReadObjectMethod and - sink = ma.getQualifier() + sink = ma.getQualifier() and + not exists(DataFlow::ExprNode node | + node.getExpr() = sink and + node + .getTypeBound() + .(RefType) + .hasQualifiedName("org.apache.commons.io.serialization", "ValidatingObjectInputStream") + ) or m instanceof XMLDecoderReadObjectMethod and sink = ma.getQualifier()