Add System.getProperties().getProperty support

2026-04-28 02:05:14 +02:00 · 2022-03-03 20:08:38 -05:00
parent 31527a67e5
commit 7ab193dde2
10 changed files with 130 additions and 7 deletions
--- a/java/ql/lib/semmle/code/java/JDK.qll
+++ b/java/ql/lib/semmle/code/java/JDK.qll
@@ -4,6 +4,7 @@

 import Member
 import semmle.code.java.security.ExternalProcess
+private import semmle.code.java.dataflow.FlowSteps

 // --- Standard types ---
 /** The class `java.lang.Object`. */
@@ -249,11 +250,13 @@ class MethodSystemGetenv extends Method {
 /**
 * Any method named `getProperty` on class `java.lang.System`.
 */
-class MethodSystemGetProperty extends Method {
+class MethodSystemGetProperty extends ValuePreservingMethod {
  MethodSystemGetProperty() {
    this.hasName("getProperty") and
    this.getDeclaringType() instanceof TypeSystem
  }
+
+  override predicate returnsValue(int arg) { arg = 1 }
 }

 /**
--- a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll
@@ -6,6 +6,7 @@ import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.DefUse
+import semmle.code.java.environment.SystemProperty
 import semmle.code.java.frameworks.Jdbc
 import semmle.code.java.frameworks.Networking
 import semmle.code.java.frameworks.Properties
@@ -182,6 +183,8 @@ class EnvInput extends LocalUserInput {
    // Results from various specific methods.
    this.asExpr().(MethodAccess).getMethod() instanceof EnvReadMethod
    or
+    this.asExpr() = getSystemProperty(_)
+    or
    // Access to `System.in`.
    exists(Field f | this.asExpr() = f.getAnAccess() | f instanceof SystemIn)
    or
@@ -203,6 +206,7 @@ class EnvReadMethod extends Method {
  EnvReadMethod() {
    this instanceof MethodSystemGetenv or
    this instanceof PropertiesGetPropertyMethod or
+    this instanceof PropertiesGetMethod or
    this instanceof MethodSystemGetProperty
  }
 }
--- a/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll
@@ -15,6 +15,7 @@ private module Frameworks {
  private import semmle.code.java.frameworks.android.Intent
  private import semmle.code.java.frameworks.android.SQLite
  private import semmle.code.java.frameworks.Guice
+  private import semmle.code.java.frameworks.Properties
  private import semmle.code.java.frameworks.Protobuf
  private import semmle.code.java.frameworks.guava.Guava
  private import semmle.code.java.frameworks.apache.Lang
--- a/java/ql/lib/semmle/code/java/environment/SystemProperty.qll
+++ b/java/ql/lib/semmle/code/java/environment/SystemProperty.qll
@@ -1,4 +1,6 @@
 import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.frameworks.Properties
 private import semmle.code.java.frameworks.apache.Lang

 /**
@@ -6,6 +8,7 @@ private import semmle.code.java.frameworks.apache.Lang
 */
 Expr getSystemProperty(string propertyName) {
  result = getSystemPropertyFromSystem(propertyName) or
+  result = getSystemPropertyFromSystemGetProperties(propertyName) or
  result = getSystemPropertyFromFile(propertyName) or
  result = getSystemPropertyFromApacheSystemUtils(propertyName) or
  result = getSystemPropertyFromApacheFileUtils(propertyName) or
@@ -15,15 +18,31 @@ Expr getSystemProperty(string propertyName) {
 }

 private MethodAccess getSystemPropertyFromSystem(string propertyName) {
-  result =
-    any(MethodAccessSystemGetProperty methodAccessSystemGetProperty |
-      methodAccessSystemGetProperty.hasCompileTimeConstantGetPropertyName(propertyName)
-    )
+  result.(MethodAccessSystemGetProperty).hasCompileTimeConstantGetPropertyName(propertyName)
  or
  exists(Method m | result.getMethod() = m | m.hasName("lineSeparator")) and
  propertyName = "line.separator"
 }

+/**
+ * A method access that retrieves the value of `propertyName` from the following methods:
+ *  - `System.getProperties().getProperty(...)`
+ *  - `System.getProperties().get(...)`
+ */
+private MethodAccess getSystemPropertyFromSystemGetProperties(string propertyName) {
+  exists(Method getMethod |
+    getMethod instanceof PropertiesGetMethod
+    or
+    getMethod instanceof PropertiesGetPropertyMethod and
+    result.getMethod() = getMethod
+  ) and
+  result.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName and
+  DataFlow::localExprFlow(any(MethodAccess m |
+      m.getMethod().getDeclaringType() instanceof TypeSystem and
+      m.getMethod().hasName("getProperties")
+    ), result.getQualifier())
+}
+
 private FieldAccess getSystemPropertyFromFile(string propertyName) {
  result.getField() instanceof FieldFileSeparator and propertyName = "file.separator"
  or
--- a/java/ql/lib/semmle/code/java/frameworks/Properties.qll
+++ b/java/ql/lib/semmle/code/java/frameworks/Properties.qll
@@ -1,15 +1,25 @@
 /* Definitions related to `java.util.Properties`. */
 import semmle.code.java.Type
+private import semmle.code.java.dataflow.FlowSteps

 library class TypeProperty extends Class {
  TypeProperty() { hasQualifiedName("java.util", "Properties") }
 }

-library class PropertiesGetPropertyMethod extends Method {
+library class PropertiesGetPropertyMethod extends ValuePreservingMethod {
  PropertiesGetPropertyMethod() {
    getDeclaringType() instanceof TypeProperty and
    hasName("getProperty")
  }
+
+  override predicate returnsValue(int arg) { arg = 1 }
+}
+
+library class PropertiesGetMethod extends Method {
+  PropertiesGetMethod() {
+    getDeclaringType() instanceof TypeProperty and
+    hasName("get")
+  }
 }

 library class PropertiesSetPropertyMethod extends Method {
--- a/java/ql/src/change-notes/2022-02-14-os-guards.md
+++ b/java/ql/src/change-notes/2022-02-14-os-guards.md
@@ -1,6 +1,8 @@
 ---
 category: minorAnalysis
 ---
- * Add new guards `IsWindowsGuard` and `IsUnixGuard` to detect OS specific guards.
+ * Add new guards `IsWindowsGuard`, `IsSpecificWindowsVariant`, `IsUnixGuard`, and `IsSpecificUnixVariant` to detect OS specific guards.
+ * Add new predicate `getSystemProperty` that gets all expressions that retrieve system properties from a variety of sources (eg. alternative JDK API's, Google Guava, Apache Commons, Apache IO, ect..).
 * Update "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) to remove false-positives when OS is properly used as logical guard.
+ * Update "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) to use `getSystemProperty` to resolve more 
 
--- a/java/ql/test/library-tests/environment/SystemPropertyAccess.java
+++ b/java/ql/test/library-tests/environment/SystemPropertyAccess.java
@@ -0,0 +1,25 @@
+import java.io.File;
+import java.util.Properties;
+import org.apache.commons.lang3.SystemUtils;
+
+public class SystemPropertyAccess {
+    private static final Properties SYSTEM_PROPERTIES = System.getProperties();
+
+    void test() {
+        System.getProperty("os.name");
+        System.getProperty("os.name", "default");
+        System.getProperties().getProperty("os.name");
+        System.getProperties().get("java.io.tmpdir");
+        SYSTEM_PROPERTIES.getProperty("java.home");
+        SYSTEM_PROPERTIES.get("file.encoding");
+        System.lineSeparator();
+        String awtToolkit = SystemUtils.AWT_TOOLKIT;
+        String fileEncoding = SystemUtils.FILE_ENCODING;
+        String tmpDir = SystemUtils.JAVA_IO_TMPDIR;
+        String separator = File.separator;
+        char separatorChar = File.separatorChar;
+        String pathSeparator = File.pathSeparator;
+        char pathSeparatorChar = File.pathSeparatorChar;
+    }
+    
+}
--- a/java/ql/test/library-tests/environment/SystemPropertyTest.expected
+++ b/java/ql/test/library-tests/environment/SystemPropertyTest.expected
@@ -0,0 +1,52 @@
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:93:5:93:50 | AWT_TOOLKIT | awt.toolkit |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:115:5:115:52 | FILE_ENCODING | file.encoding |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:141:5:142:53 | FILE_SEPARATOR | file.separator |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:160:5:160:53 | JAVA_AWT_FONTS | java.awt.fonts |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:178:5:178:59 | JAVA_AWT_GRAPHICSENV | java.awt.graphicsenv |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:199:5:199:56 | JAVA_AWT_HEADLESS | java.awt.headless |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:217:5:217:58 | JAVA_AWT_PRINTERJOB | java.awt.printerjob |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:235:5:235:54 | JAVA_CLASS_PATH | java.class.path |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:253:5:253:57 | JAVA_CLASS_VERSION | java.class.version |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:272:5:272:52 | JAVA_COMPILER | java.compiler |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:308:5:308:52 | JAVA_EXT_DIRS | java.ext.dirs |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:326:5:326:48 | JAVA_HOME | java.home |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:344:5:344:53 | JAVA_IO_TMPDIR | java.io.tmpdir |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:362:5:362:56 | JAVA_LIBRARY_PATH | java.library.path |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:381:5:381:56 | JAVA_RUNTIME_NAME | java.runtime.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:400:5:400:59 | JAVA_RUNTIME_VERSION | java.runtime.version |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:418:5:418:62 | JAVA_SPECIFICATION_NAME | java.specification.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:436:5:436:64 | JAVA_SPECIFICATION_VENDOR | java.specification.vendor |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:473:5:474:13 | JAVA_UTIL_PREFS_PREFERENCES_FACTORY | java.util.prefs.PreferencesFactory |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:492:5:492:50 | JAVA_VENDOR | java.vendor |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:510:5:510:54 | JAVA_VENDOR_URL | java.vendor.url |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:528:5:528:51 | JAVA_VERSION | java.version |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:547:5:547:51 | JAVA_VM_INFO | java.vm.info |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:565:5:565:51 | JAVA_VM_NAME | java.vm.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:583:5:583:65 | JAVA_VM_SPECIFICATION_NAME | java.vm.specification.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:601:5:601:67 | JAVA_VM_SPECIFICATION_VENDOR | java.vm.specification.vendor |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:637:5:637:53 | JAVA_VM_VENDOR | java.vm.vendor |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:655:5:655:54 | JAVA_VM_VERSION | java.vm.version |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:674:5:675:53 | LINE_SEPARATOR | line.separator |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:693:5:693:46 | OS_ARCH | os.arch |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:711:5:711:46 | OS_NAME | os.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:729:5:729:49 | OS_VERSION | os.version |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:749:5:750:53 | PATH_SEPARATOR | path.separator |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:770:5:770:73 | USER_COUNTRY | user.country |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:788:5:788:47 | USER_DIR | user.dir |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:806:5:806:48 | USER_HOME | user.home |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:825:5:825:52 | USER_LANGUAGE | user.language |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:843:5:843:48 | USER_NAME | user.name |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:861:5:861:52 | USER_TIMEZONE | user.timezone |
+| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1762:47:1762:63 | JAVA_AWT_HEADLESS | java.awt.headless |
+| SystemPropertyAccess.java:9:9:9:37 | getProperty(...) | os.name |
+| SystemPropertyAccess.java:10:9:10:48 | getProperty(...) | os.name |
+| SystemPropertyAccess.java:11:9:11:53 | getProperty(...) | os.name |
+| SystemPropertyAccess.java:12:9:12:52 | get(...) | java.io.tmpdir |
+| SystemPropertyAccess.java:15:9:15:30 | lineSeparator(...) | line.separator |
+| SystemPropertyAccess.java:16:29:16:51 | SystemUtils.AWT_TOOLKIT | awt.toolkit |
+| SystemPropertyAccess.java:17:31:17:55 | SystemUtils.FILE_ENCODING | file.encoding |
+| SystemPropertyAccess.java:18:25:18:50 | SystemUtils.JAVA_IO_TMPDIR | java.io.tmpdir |
+| SystemPropertyAccess.java:19:28:19:41 | File.separator | file.separator |
+| SystemPropertyAccess.java:20:30:20:47 | File.separatorChar | file.separator |
+| SystemPropertyAccess.java:21:32:21:49 | File.pathSeparator | path.separator |
+| SystemPropertyAccess.java:22:34:22:55 | File.pathSeparatorChar | path.separator |
--- a/java/ql/test/library-tests/environment/SystemPropertyTest.ql
+++ b/java/ql/test/library-tests/environment/SystemPropertyTest.ql
@@ -0,0 +1,6 @@
+import default
+import semmle.code.java.environment.SystemProperty
+
+from Expr systemPropertyAccess, string propertyName
+where systemPropertyAccess = getSystemProperty(propertyName)
+select systemPropertyAccess, propertyName
--- a/java/ql/test/library-tests/environment/options
+++ b/java/ql/test/library-tests/environment/options
@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/apache-commons-lang3-3.7/
				`@@ -0,0 +1 @@`
				`//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/apache-commons-lang3-3.7/`