Skip to content

Commit 8a8f0fc

Browse files
JLLeitschuhJLLeitschuh
committed
Improve system property lookup
1 parent 6e30e50 commit 8a8f0fc

File tree

5 files changed

+138
-32
lines changed

5 files changed

+138
-32
lines changed

‎java/ql/lib/semmle/code/java/JDK.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ class MethodAccessSystemGetProperty extends MethodAccess {
265265
/**
266266
* Holds if this call has a compile-time constant first argument with the value `propertyName`.
267267
* For example: `System.getProperty("user.dir")`.
268+
*
269+
* Note: Better to use `semmle.code.java.environment.SystemProperty#getSystemProperty` instead.
268270
*/
269271
predicate hasCompileTimeConstantGetPropertyName(string propertyName) {
270272
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName

‎java/ql/lib/semmle/code/java/environment/SystemProperty.qll

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import java
2+
private import semmle.code.java.frameworks.apache.Lang
3+
4+
Expr getSystemProperty(string propertyName) {
5+
result =
6+
any(MethodAccessSystemGetProperty methodAccessSystemGetProperty |
7+
methodAccessSystemGetProperty.hasCompileTimeConstantGetPropertyName(propertyName)
8+
) or
9+
result = getSystemPropertyFromApacheSystemUtils(propertyName) or
10+
result = getSystemPropertyFromApacheFileUtils(propertyName) or
11+
result = getSystemPropertyFromOperatingSystemMXBean(propertyName)
12+
}
13+
14+
/**
15+
* A field access to the system property.
16+
* See: https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/SystemUtils.html
17+
*/
18+
private FieldAccess getSystemPropertyFromApacheSystemUtils(string propertyName) {
19+
exists(Field f | f = result.getField() and f.getDeclaringType() instanceof ApacheSystemUtils |
20+
f.getName() = "AWT_TOOLKIT" and propertyName = "awt.toolkit"
21+
or
22+
f.getName() = "FILE_ENCODING" and propertyName = "file.encoding"
23+
or
24+
f.getName() = "FILE_SEPARATOR" and propertyName = "file.separator"
25+
or
26+
f.getName() = "JAVA_AWT_FONTS" and propertyName = "java.awt.fonts"
27+
or
28+
f.getName() = "JAVA_AWT_GRAPHICSENV" and propertyName = "java.awt.graphicsenv"
29+
or
30+
f.getName() = "JAVA_AWT_HEADLESS" and propertyName = "java.awt.headless"
31+
or
32+
f.getName() = "JAVA_AWT_PRINTERJOB" and propertyName = "java.awt.printerjob"
33+
or
34+
f.getName() = "JAVA_CLASS_PATH" and propertyName = "java.class.path"
35+
or
36+
f.getName() = "JAVA_CLASS_VERSION" and propertyName = "java.class.version"
37+
or
38+
f.getName() = "JAVA_COMPILER" and propertyName = "java.compiler"
39+
or
40+
f.getName() = "JAVA_EXT_DIRS" and propertyName = "java.ext.dirs"
41+
or
42+
f.getName() = "JAVA_HOME" and propertyName = "java.home"
43+
or
44+
f.getName() = "JAVA_IO_TMPDIR" and propertyName = "java.io.tmpdir"
45+
or
46+
f.getName() = "JAVA_LIBRARY_PATH" and propertyName = "java.library.path"
47+
or
48+
f.getName() = "JAVA_RUNTIME_NAME" and propertyName = "java.runtime.name"
49+
or
50+
f.getName() = "JAVA_RUNTIME_VERSION" and propertyName = "java.runtime.version"
51+
or
52+
f.getName() = "JAVA_SPECIFICATION_NAME" and propertyName = "java.specification.name"
53+
or
54+
f.getName() = "JAVA_SPECIFICATION_VENDOR" and propertyName = "java.specification.vendor"
55+
or
56+
f.getName() = "JAVA_UTIL_PREFS_PREFERENCES_FACTORY" and
57+
propertyName = "java.util.prefs.PreferencesFactory"
58+
or
59+
f.getName() = "JAVA_VENDOR" and propertyName = "java.vendor"
60+
or
61+
f.getName() = "JAVA_VENDOR_URL" and propertyName = "java.vendor.url"
62+
or
63+
f.getName() = "JAVA_VERSION" and propertyName = "java.version"
64+
or
65+
f.getName() = "JAVA_VM_INFO" and propertyName = "java.vm.info"
66+
or
67+
f.getName() = "JAVA_VM_NAME" and propertyName = "java.vm.name"
68+
or
69+
f.getName() = "JAVA_VM_SPECIFICATION_NAME" and propertyName = "java.vm.specification.name"
70+
or
71+
f.getName() = "JAVA_VM_SPECIFICATION_VENDOR" and propertyName = "java.vm.specification.vendor"
72+
or
73+
f.getName() = "JAVA_VM_VENDOR" and propertyName = "java.vm.vendor"
74+
or
75+
f.getName() = "JAVA_VM_VERSION" and propertyName = "java.vm.version"
76+
or
77+
f.getName() = "LINE_SEPARATOR" and propertyName = "line.separator"
78+
or
79+
f.getName() = "OS_ARCH" and propertyName = "os.arch"
80+
or
81+
f.getName() = "OS_NAME" and propertyName = "os.name"
82+
or
83+
f.getName() = "OS_VERSION" and propertyName = "os.version"
84+
or
85+
f.getName() = "PATH_SEPARATOR" and propertyName = "path.separator"
86+
or
87+
f.getName() = "USER_COUNTRY" and propertyName = "user.country"
88+
or
89+
f.getName() = "USER_DIR" and propertyName = "user.dir"
90+
or
91+
f.getName() = "USER_HOME" and propertyName = "user.home"
92+
or
93+
f.getName() = "USER_LANGUAGE" and propertyName = "user.language"
94+
or
95+
f.getName() = "USER_NAME" and propertyName = "user.name"
96+
or
97+
f.getName() = "USER_TIMEZONE" and propertyName = "user.timezone"
98+
)
99+
}
100+
101+
private MethodAccess getSystemPropertyFromApacheFileUtils(string propertyName) {
102+
exists(Method m |
103+
result.getMethod() = m and
104+
m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils")
105+
|
106+
m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and propertyName = "java.io.tmpdir"
107+
or
108+
m.hasName(["getUserDirectory", "getUserDirectoryPath"]) and propertyName = "user.home"
109+
)
110+
}
111+
112+
private MethodAccess getSystemPropertyFromOperatingSystemMXBean(string propertyName) {
113+
exists(Method m |
114+
m = result.getMethod() and
115+
m.getDeclaringType().hasQualifiedName("java.lang.management", "OperatingSystemMXBean")
116+
|
117+
m.getName() = "getName" and propertyName = "os.name"
118+
or
119+
m.getName() = "getArch" and propertyName = "os.arch"
120+
or
121+
m.getName() = "getVersion" and propertyName = "os.version"
122+
)
123+
}

‎java/ql/lib/semmle/code/java/os/OSCheck.qll

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import java
66
import semmle.code.java.controlflow.Guards
7+
private import semmle.code.java.environment.SystemProperty
78
private import semmle.code.java.frameworks.apache.Lang
89
private import semmle.code.java.dataflow.DataFlow
910

@@ -40,21 +41,21 @@ abstract class IsAnyUnixGuard extends Guard { }
4041
*/
4142
bindingset[osString]
4243
private predicate isOsFromSystemProp(MethodAccess ma, string osString) {
43-
exists(MethodAccessSystemGetProperty sgpMa, Expr sgpFlowsToExpr |
44-
sgpMa.hasCompileTimeConstantGetPropertyName("os.name")
44+
exists(Expr systemGetPropertyExpr, Expr systemGetPropertyFlowsToExpr |
45+
systemGetPropertyExpr = getSystemProperty("os.name")
4546
|
46-
DataFlow::localExprFlow(sgpMa, sgpFlowsToExpr) and
47+
DataFlow::localExprFlow(systemGetPropertyExpr, systemGetPropertyFlowsToExpr) and
4748
ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches(osString) and // Call from System.getProperty to some partial match method
4849
(
49-
sgpFlowsToExpr = ma.getQualifier()
50+
systemGetPropertyFlowsToExpr = ma.getQualifier()
5051
or
5152
exists(MethodAccess caseChangeMa |
5253
caseChangeMa.getMethod() =
5354
any(Method m |
5455
m.getDeclaringType() instanceof TypeString and m.hasName(["toLowerCase", "toUpperCase"])
5556
)
5657
|
57-
sgpFlowsToExpr = caseChangeMa.getQualifier() and // Call from System.getProperty to case-switching method
58+
systemGetPropertyFlowsToExpr = caseChangeMa.getQualifier() and // Call from System.getProperty to case-switching method
5859
DataFlow::localExprFlow(caseChangeMa, ma.getQualifier()) // Call from case-switching method to some partial match method
5960
)
6061
)

‎java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
132132
TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
133133

134134
override predicate isSource(DataFlow::Node source) {
135-
source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
135+
source.asExpr() instanceof ExprSystemGetPropertyTempDirTainted
136136
}
137137

138138
/**
@@ -178,9 +178,9 @@ private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTrackin
178178

179179
override predicate isSource(DataFlow::Node node) {
180180
exists(
181-
MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite
181+
ExprSystemGetPropertyTempDirTainted propertyGetExpr, DataFlow::Node callSite
182182
|
183-
DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite)
183+
DataFlow::localFlow(DataFlow::exprNode(propertyGetExpr), callSite)
184184
|
185185
isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1)
186186
)

‎java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,14 @@
33
*/
44

55
import java
6+
private import semmle.code.java.environment.SystemProperty
67
import semmle.code.java.dataflow.FlowSources
78

89
/**
9-
* A method that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
10+
* A method or field access that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
1011
*/
11-
abstract class MethodAccessSystemGetPropertyTempDirTainted extends MethodAccess { }
12-
13-
/**
14-
* Method access `System.getProperty("java.io.tmpdir")`.
15-
*/
16-
private class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetPropertyTempDirTainted,
17-
MethodAccessSystemGetProperty {
18-
MethodAccessSystemGetPropertyTempDir() {
19-
this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir")
20-
}
21-
}
22-
23-
/**
24-
* A method call to the `org.apache.commons.io.FileUtils` methods `getTempDirectory` or `getTempDirectoryPath`.
25-
*/
26-
private class MethodAccessApacheFileUtilsTempDir extends MethodAccessSystemGetPropertyTempDirTainted {
27-
MethodAccessApacheFileUtilsTempDir() {
28-
exists(Method m |
29-
m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils") and
30-
m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and
31-
this.getMethod() = m
32-
)
33-
}
12+
class ExprSystemGetPropertyTempDirTainted extends Expr {
13+
ExprSystemGetPropertyTempDirTainted() { this = getSystemProperty("java.io.tmpdir") }
3414
}
3515

3616
/**

0 commit comments

Comments
 (0)