From 54950c2f42ca70e2b59ce3ae4fed266c87d3a674 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Fri, 1 Jan 2021 20:07:45 -0500 Subject: [PATCH 1/3] Add MethodAccessSystemGetProperty predicate --- java/ql/src/semmle/code/java/JDK.qll | 15 +++++++++++++ java/ql/src/semmle/code/java/PrintAst.ql | 3 ++- .../JDK/SystemGetPropertyCall.expected | 3 +++ .../JDK/SystemGetPropertyCall.ql | 10 +++++++++ .../JDK/jdk/SystemGetPropertyCall.java | 21 +++++++++++++++++++ 5 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 java/ql/test/library-tests/JDK/SystemGetPropertyCall.expected create mode 100644 java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql create mode 100644 java/ql/test/library-tests/JDK/jdk/SystemGetPropertyCall.java diff --git a/java/ql/src/semmle/code/java/JDK.qll b/java/ql/src/semmle/code/java/JDK.qll index d9a1a15e5d3d..78a326bb9b45 100644 --- a/java/ql/src/semmle/code/java/JDK.qll +++ b/java/ql/src/semmle/code/java/JDK.qll @@ -211,6 +211,21 @@ class MethodSystemGetProperty extends Method { } } +/** + * Any method access to a method named `getProperty` on class `java.lang.System`. + */ +class MethodAccessSystemGetProperty extends MethodAccess { + MethodAccessSystemGetProperty() { getMethod() instanceof MethodSystemGetProperty } + + /** + * Holds true if this is a compile-time constant call for the specified `propertyName`. + * Eg. `System.getProperty("user.dir")`. + */ + predicate hasCompileTimeConstantGetPropertyName(string propertyName) { + this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName + } +} + /** * Any method named `exit` on class `java.lang.Runtime` or `java.lang.System`. */ diff --git a/java/ql/src/semmle/code/java/PrintAst.ql b/java/ql/src/semmle/code/java/PrintAst.ql index 3931a9670e61..27e630d69364 100644 --- a/java/ql/src/semmle/code/java/PrintAst.ql +++ b/java/ql/src/semmle/code/java/PrintAst.ql @@ -16,5 +16,6 @@ class PrintAstConfigurationOverride extends PrintAstConfiguration { /** * TWEAK THIS PREDICATE AS NEEDED. */ - override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) } + override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) and + not l.getFile().getBaseName().matches("SystemGetPropertyCall.java") } } diff --git a/java/ql/test/library-tests/JDK/SystemGetPropertyCall.expected b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.expected new file mode 100644 index 000000000000..e96a74fe1a6a --- /dev/null +++ b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.expected @@ -0,0 +1,3 @@ +| jdk/SystemGetPropertyCall.java:7:9:7:38 | getProperty(...) | +| jdk/SystemGetPropertyCall.java:11:9:11:46 | getProperty(...) | +| jdk/SystemGetPropertyCall.java:15:9:15:45 | getProperty(...) | diff --git a/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql new file mode 100644 index 000000000000..2dd2979ad41d --- /dev/null +++ b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql @@ -0,0 +1,10 @@ +/** + * @name SystemCall + * @description Test the definition of System Get Property + */ + +import default + +from MethodAccessSystemGetProperty ma +where ma.hasCompileTimeConstantGetPropertyName("user.dir") +select ma diff --git a/java/ql/test/library-tests/JDK/jdk/SystemGetPropertyCall.java b/java/ql/test/library-tests/JDK/jdk/SystemGetPropertyCall.java new file mode 100644 index 000000000000..750f1e8b83c0 --- /dev/null +++ b/java/ql/test/library-tests/JDK/jdk/SystemGetPropertyCall.java @@ -0,0 +1,21 @@ +package jdk; + +public class SystemGetPropertyCall { + private static final String USER_DIR_PROPERTY = "user.dir"; + + void a() { + System.getProperty("user.dir"); + } + + void b() { + System.getProperty("user.dir", "HOME"); + } + + void c() { + System.getProperty(USER_DIR_PROPERTY); + } + + void d() { + System.getProperty("random.property"); + } +} From 028e4756bb0ec34a183fa3b621f0d88b903f35b9 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Mon, 4 Jan 2021 10:13:52 -0500 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Anders Schack-Mulligen --- java/ql/src/semmle/code/java/JDK.qll | 6 +++--- java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql | 7 +------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/java/ql/src/semmle/code/java/JDK.qll b/java/ql/src/semmle/code/java/JDK.qll index 78a326bb9b45..3b71396c9207 100644 --- a/java/ql/src/semmle/code/java/JDK.qll +++ b/java/ql/src/semmle/code/java/JDK.qll @@ -212,14 +212,14 @@ class MethodSystemGetProperty extends Method { } /** - * Any method access to a method named `getProperty` on class `java.lang.System`. + * An access to a method named `getProperty` on class `java.lang.System`. */ class MethodAccessSystemGetProperty extends MethodAccess { MethodAccessSystemGetProperty() { getMethod() instanceof MethodSystemGetProperty } /** - * Holds true if this is a compile-time constant call for the specified `propertyName`. - * Eg. `System.getProperty("user.dir")`. + * Holds if this call has a compile-time constant first argument with the value `propertyName`. + * For example: `System.getProperty("user.dir")`. */ predicate hasCompileTimeConstantGetPropertyName(string propertyName) { this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName diff --git a/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql index 2dd2979ad41d..4ccacefa2c27 100644 --- a/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql +++ b/java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql @@ -1,9 +1,4 @@ -/** - * @name SystemCall - * @description Test the definition of System Get Property - */ - -import default +import java from MethodAccessSystemGetProperty ma where ma.hasCompileTimeConstantGetPropertyName("user.dir") From ba4a562c9a272bac132f2f453e2f9fff0b8b5a39 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Mon, 4 Jan 2021 21:31:42 -0500 Subject: [PATCH 3/3] Update PrintAst.actual with new test output --- java/ql/src/semmle/code/java/PrintAst.ql | 3 +- .../test/library-tests/JDK/PrintAst.expected | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/java/ql/src/semmle/code/java/PrintAst.ql b/java/ql/src/semmle/code/java/PrintAst.ql index 27e630d69364..3931a9670e61 100644 --- a/java/ql/src/semmle/code/java/PrintAst.ql +++ b/java/ql/src/semmle/code/java/PrintAst.ql @@ -16,6 +16,5 @@ class PrintAstConfigurationOverride extends PrintAstConfiguration { /** * TWEAK THIS PREDICATE AS NEEDED. */ - override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) and - not l.getFile().getBaseName().matches("SystemGetPropertyCall.java") } + override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) } } diff --git a/java/ql/test/library-tests/JDK/PrintAst.expected b/java/ql/test/library-tests/JDK/PrintAst.expected index ff260ac6c38a..74967e122e30 100644 --- a/java/ql/test/library-tests/JDK/PrintAst.expected +++ b/java/ql/test/library-tests/JDK/PrintAst.expected @@ -60,3 +60,38 @@ jdk/A.java: # 28| 0: [ArrayTypeAccess] ...[] # 28| 0: [TypeAccess] String # 28| 5: [BlockStmt] stmt +jdk/SystemGetPropertyCall.java: +# 0| [CompilationUnit] SystemGetPropertyCall +# 3| 1: [Class] SystemGetPropertyCall +# 4| 3: [FieldDeclaration] String USER_DIR_PROPERTY, ...; +# 4| -1: [TypeAccess] String +# 4| 0: [StringLiteral] "user.dir" +# 6| 4: [Method] a +# 6| 3: [TypeAccess] void +# 6| 5: [BlockStmt] stmt +# 7| 0: [ExprStmt] stmt +# 7| 0: [MethodAccess] getProperty(...) +# 7| -1: [TypeAccess] System +# 7| 0: [StringLiteral] "user.dir" +# 10| 5: [Method] b +# 10| 3: [TypeAccess] void +# 10| 5: [BlockStmt] stmt +# 11| 0: [ExprStmt] stmt +# 11| 0: [MethodAccess] getProperty(...) +# 11| -1: [TypeAccess] System +# 11| 0: [StringLiteral] "user.dir" +# 11| 1: [StringLiteral] "HOME" +# 14| 6: [Method] c +# 14| 3: [TypeAccess] void +# 14| 5: [BlockStmt] stmt +# 15| 0: [ExprStmt] stmt +# 15| 0: [MethodAccess] getProperty(...) +# 15| -1: [TypeAccess] System +# 15| 0: [VarAccess] USER_DIR_PROPERTY +# 18| 7: [Method] d +# 18| 3: [TypeAccess] void +# 18| 5: [BlockStmt] stmt +# 19| 0: [ExprStmt] stmt +# 19| 0: [MethodAccess] getProperty(...) +# 19| -1: [TypeAccess] System +# 19| 0: [StringLiteral] "random.property"