Skip to content

Commit

Permalink
Improve system property lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
JLLeitschuh committed Mar 2, 2022
1 parent dad9a02 commit 82d3cd8
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 32 deletions.
2 changes: 2 additions & 0 deletions java/ql/lib/semmle/code/java/JDK.qll
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ class MethodAccessSystemGetProperty extends MethodAccess {
/**
* Holds if this call has a compile-time constant first argument with the value `propertyName`.
* For example: `System.getProperty("user.dir")`.
*
* Note: Better to use `semmle.code.java.environment.SystemProperty#getSystemProperty` instead.
*/
predicate hasCompileTimeConstantGetPropertyName(string propertyName) {
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName
Expand Down
123 changes: 123 additions & 0 deletions java/ql/lib/semmle/code/java/environment/SystemProperty.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import java
private import semmle.code.java.frameworks.apache.Lang

Expr getSystemProperty(string propertyName) {
result =
any(MethodAccessSystemGetProperty methodAccessSystemGetProperty |
methodAccessSystemGetProperty.hasCompileTimeConstantGetPropertyName(propertyName)
) or
result = getSystemPropertyFromApacheSystemUtils(propertyName) or
result = getSystemPropertyFromApacheFileUtils(propertyName) or
result = getSystemPropertyFromOperatingSystemMXBean(propertyName)
}

/**
* A field access to the system property.
* See: https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/SystemUtils.html
*/
private FieldAccess getSystemPropertyFromApacheSystemUtils(string propertyName) {
exists(Field f | f = result.getField() and f.getDeclaringType() instanceof ApacheSystemUtils |
f.getName() = "AWT_TOOLKIT" and propertyName = "awt.toolkit"
or
f.getName() = "FILE_ENCODING" and propertyName = "file.encoding"
or
f.getName() = "FILE_SEPARATOR" and propertyName = "file.separator"
or
f.getName() = "JAVA_AWT_FONTS" and propertyName = "java.awt.fonts"
or
f.getName() = "JAVA_AWT_GRAPHICSENV" and propertyName = "java.awt.graphicsenv"
or
f.getName() = "JAVA_AWT_HEADLESS" and propertyName = "java.awt.headless"
or
f.getName() = "JAVA_AWT_PRINTERJOB" and propertyName = "java.awt.printerjob"
or
f.getName() = "JAVA_CLASS_PATH" and propertyName = "java.class.path"
or
f.getName() = "JAVA_CLASS_VERSION" and propertyName = "java.class.version"
or
f.getName() = "JAVA_COMPILER" and propertyName = "java.compiler"
or
f.getName() = "JAVA_EXT_DIRS" and propertyName = "java.ext.dirs"
or
f.getName() = "JAVA_HOME" and propertyName = "java.home"
or
f.getName() = "JAVA_IO_TMPDIR" and propertyName = "java.io.tmpdir"
or
f.getName() = "JAVA_LIBRARY_PATH" and propertyName = "java.library.path"
or
f.getName() = "JAVA_RUNTIME_NAME" and propertyName = "java.runtime.name"
or
f.getName() = "JAVA_RUNTIME_VERSION" and propertyName = "java.runtime.version"
or
f.getName() = "JAVA_SPECIFICATION_NAME" and propertyName = "java.specification.name"
or
f.getName() = "JAVA_SPECIFICATION_VENDOR" and propertyName = "java.specification.vendor"
or
f.getName() = "JAVA_UTIL_PREFS_PREFERENCES_FACTORY" and
propertyName = "java.util.prefs.PreferencesFactory"
or
f.getName() = "JAVA_VENDOR" and propertyName = "java.vendor"
or
f.getName() = "JAVA_VENDOR_URL" and propertyName = "java.vendor.url"
or
f.getName() = "JAVA_VERSION" and propertyName = "java.version"
or
f.getName() = "JAVA_VM_INFO" and propertyName = "java.vm.info"
or
f.getName() = "JAVA_VM_NAME" and propertyName = "java.vm.name"
or
f.getName() = "JAVA_VM_SPECIFICATION_NAME" and propertyName = "java.vm.specification.name"
or
f.getName() = "JAVA_VM_SPECIFICATION_VENDOR" and propertyName = "java.vm.specification.vendor"
or
f.getName() = "JAVA_VM_VENDOR" and propertyName = "java.vm.vendor"
or
f.getName() = "JAVA_VM_VERSION" and propertyName = "java.vm.version"
or
f.getName() = "LINE_SEPARATOR" and propertyName = "line.separator"
or
f.getName() = "OS_ARCH" and propertyName = "os.arch"
or
f.getName() = "OS_NAME" and propertyName = "os.name"
or
f.getName() = "OS_VERSION" and propertyName = "os.version"
or
f.getName() = "PATH_SEPARATOR" and propertyName = "path.separator"
or
f.getName() = "USER_COUNTRY" and propertyName = "user.country"
or
f.getName() = "USER_DIR" and propertyName = "user.dir"
or
f.getName() = "USER_HOME" and propertyName = "user.home"
or
f.getName() = "USER_LANGUAGE" and propertyName = "user.language"
or
f.getName() = "USER_NAME" and propertyName = "user.name"
or
f.getName() = "USER_TIMEZONE" and propertyName = "user.timezone"
)
}

private MethodAccess getSystemPropertyFromApacheFileUtils(string propertyName) {
exists(Method m |
result.getMethod() = m and
m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils")
|
m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and propertyName = "java.io.tmpdir"
or
m.hasName(["getUserDirectory", "getUserDirectoryPath"]) and propertyName = "user.home"
)
}

private MethodAccess getSystemPropertyFromOperatingSystemMXBean(string propertyName) {
exists(Method m |
m = result.getMethod() and
m.getDeclaringType().hasQualifiedName("java.lang.management", "OperatingSystemMXBean")
|
m.getName() = "getName" and propertyName = "os.name"
or
m.getName() = "getArch" and propertyName = "os.arch"
or
m.getName() = "getVersion" and propertyName = "os.version"
)
}
11 changes: 6 additions & 5 deletions java/ql/lib/semmle/code/java/os/OSCheck.qll
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java
import semmle.code.java.controlflow.Guards
private import semmle.code.java.environment.SystemProperty
private import semmle.code.java.frameworks.apache.Lang
private import semmle.code.java.dataflow.DataFlow

Expand Down Expand Up @@ -40,21 +41,21 @@ abstract class IsAnyUnixGuard extends Guard { }
*/
bindingset[osString]
private predicate isOsFromSystemProp(MethodAccess ma, string osString) {
exists(MethodAccessSystemGetProperty sgpMa, Expr sgpFlowsToExpr |
sgpMa.hasCompileTimeConstantGetPropertyName("os.name")
exists(Expr systemGetPropertyExpr, Expr systemGetPropertyFlowsToExpr |
systemGetPropertyExpr = getSystemProperty("os.name")
|
DataFlow::localExprFlow(sgpMa, sgpFlowsToExpr) and
DataFlow::localExprFlow(systemGetPropertyExpr, systemGetPropertyFlowsToExpr) and
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches(osString) and // Call from System.getProperty to some partial match method
(
sgpFlowsToExpr = ma.getQualifier()
systemGetPropertyFlowsToExpr = ma.getQualifier()
or
exists(MethodAccess caseChangeMa |
caseChangeMa.getMethod() =
any(Method m |
m.getDeclaringType() instanceof TypeString and m.hasName(["toLowerCase", "toUpperCase"])
)
|
sgpFlowsToExpr = caseChangeMa.getQualifier() and // Call from System.getProperty to case-switching method
systemGetPropertyFlowsToExpr = caseChangeMa.getQualifier() and // Call from System.getProperty to case-switching method
DataFlow::localExprFlow(caseChangeMa, ma.getQualifier()) // Call from case-switching method to some partial match method
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }

override predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
source.asExpr() instanceof ExprSystemGetPropertyTempDirTainted
}

/**
Expand Down Expand Up @@ -178,9 +178,9 @@ private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTrackin

override predicate isSource(DataFlow::Node node) {
exists(
MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite
ExprSystemGetPropertyTempDirTainted propertyGetExpr, DataFlow::Node callSite
|
DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite)
DataFlow::localFlow(DataFlow::exprNode(propertyGetExpr), callSite)
|
isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1)
)
Expand Down
28 changes: 4 additions & 24 deletions java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,14 @@
*/

import java
private import semmle.code.java.environment.SystemProperty
import semmle.code.java.dataflow.FlowSources

/**
* A method that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
* A method or field access that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
*/
abstract class MethodAccessSystemGetPropertyTempDirTainted extends MethodAccess { }

/**
* Method access `System.getProperty("java.io.tmpdir")`.
*/
private class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetPropertyTempDirTainted,
MethodAccessSystemGetProperty {
MethodAccessSystemGetPropertyTempDir() {
this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir")
}
}

/**
* A method call to the `org.apache.commons.io.FileUtils` methods `getTempDirectory` or `getTempDirectoryPath`.
*/
private class MethodAccessApacheFileUtilsTempDir extends MethodAccessSystemGetPropertyTempDirTainted {
MethodAccessApacheFileUtilsTempDir() {
exists(Method m |
m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils") and
m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and
this.getMethod() = m
)
}
class ExprSystemGetPropertyTempDirTainted extends Expr {
ExprSystemGetPropertyTempDirTainted() { this = getSystemProperty("java.io.tmpdir") }
}

/**
Expand Down

0 comments on commit 82d3cd8

Please sign in to comment.