Skip to content

Commit a7adbb7

Browse files
committed
Refactor more system property access logic
1 parent 3c53a05 commit a7adbb7

File tree

8 files changed

+182
-102
lines changed

8 files changed

+182
-102
lines changed

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -202,21 +202,6 @@ class TypeFile extends Class {
202202
TypeFile() { this.hasQualifiedName("java.io", "File") }
203203
}
204204

205-
/** The field `java.io.File.separator` or `java.io.File.separatorChar` */
206-
class FieldFileSeparator extends Field {
207-
FieldFileSeparator() {
208-
this.getDeclaringType() instanceof TypeFile and this.hasName(["separator", "separatorChar"])
209-
}
210-
}
211-
212-
/* The field `java.io.File.pathSeparator` or `java.io.File.pathSeparatorChar` */
213-
class FieldFilePathSeparator extends Field {
214-
FieldFilePathSeparator() {
215-
this.getDeclaringType() instanceof TypeFile and
216-
this.hasName(["pathSeparator", "pathSeparatorChar"])
217-
}
218-
}
219-
220205
// --- Standard methods ---
221206
/**
222207
* Any constructor of class `java.lang.ProcessBuilder`.

java/ql/lib/semmle/code/java/StringFormat.qll

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

55
import java
66
import dataflow.DefUse
7+
private import semmle.code.java.environment.SystemProperty
78

89
/**
910
* A library method that formats a number of its arguments according to a
@@ -312,24 +313,7 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
312313
or
313314
formatStringValue(e.(ChooseExpr).getAResultExpr(), fmtvalue)
314315
or
315-
exists(Method getprop, MethodAccess ma, string prop |
316-
e = ma and
317-
ma.getMethod() = getprop and
318-
getprop.hasName("getProperty") and
319-
getprop.getDeclaringType().hasQualifiedName("java.lang", "System") and
320-
getprop.getNumberOfParameters() = 1 and
321-
ma.getAnArgument().(StringLiteral).getValue() = prop and
322-
(prop = "line.separator" or prop = "file.separator" or prop = "path.separator") and
323-
fmtvalue = "x" // dummy value
324-
)
325-
or
326-
exists(Field f |
327-
e = f.getAnAccess() and
328-
fmtvalue = "x" // dummy value
329-
|
330-
f instanceof FieldFileSeparator or
331-
f instanceof FieldFilePathSeperator
332-
)
316+
e = getSystemProperty(["line.separator", "file.separator", "path.separator"]) and fmtvalue = "x" // dummy value
333317
)
334318
}
335319

Lines changed: 151 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,49 @@
11
import java
22
private import semmle.code.java.frameworks.apache.Lang
33

4+
/**
5+
* Gets Expr that return the value of `propertyName` from `System.getProperty()`.
6+
* Gets Expr that return the value of `propertyName` from `System.getProperty()`.
7+
*/
48
Expr getSystemProperty(string propertyName) {
9+
result = getSystemPropertyFromSystem(propertyName) or
10+
result = getSystemPropertyFromFile(propertyName) or
11+
result = getSystemPropertyFromApacheSystemUtils(propertyName) or
12+
result = getSystemPropertyFromApacheFileUtils(propertyName) or
13+
result = getSystemPropertyFromGuava(propertyName) or
14+
result = getSystemPropertyFromOperatingSystemMXBean(propertyName) or
15+
result = getSystemPropertyFromSpringProperties(propertyName)
16+
}
17+
18+
private MethodAccess getSystemPropertyFromSystem(string propertyName) {
519
result =
620
any(MethodAccessSystemGetProperty methodAccessSystemGetProperty |
721
methodAccessSystemGetProperty.hasCompileTimeConstantGetPropertyName(propertyName)
8-
) or
9-
result = getSystemPropertyFromApacheSystemUtils(propertyName) or
10-
result = getSystemPropertyFromApacheFileUtils(propertyName) or
11-
result = getSystemPropertyFromOperatingSystemMXBean(propertyName)
22+
)
23+
or
24+
exists(Method m | result.getMethod() = m | m.hasName("lineSeparator")) and
25+
propertyName = "line.separator"
26+
}
27+
28+
private FieldAccess getSystemPropertyFromFile(string propertyName) {
29+
result.getField() instanceof FieldFileSeparator and propertyName = "file.separator"
30+
or
31+
result.getField() instanceof FieldFilePathSeparator and propertyName = "path.separator"
32+
}
33+
34+
/** The field `java.io.File.separator` or `java.io.File.separatorChar` */
35+
private class FieldFileSeparator extends Field {
36+
FieldFileSeparator() {
37+
this.getDeclaringType() instanceof TypeFile and this.hasName(["separator", "separatorChar"])
38+
}
39+
}
40+
41+
/* The field `java.io.File.pathSeparator` or `java.io.File.pathSeparatorChar` */
42+
private class FieldFilePathSeparator extends Field {
43+
FieldFilePathSeparator() {
44+
this.getDeclaringType() instanceof TypeFile and
45+
this.hasName(["pathSeparator", "pathSeparatorChar"])
46+
}
1247
}
1348

1449
/**
@@ -17,84 +52,84 @@ Expr getSystemProperty(string propertyName) {
1752
*/
1853
private FieldAccess getSystemPropertyFromApacheSystemUtils(string propertyName) {
1954
exists(Field f | f = result.getField() and f.getDeclaringType() instanceof ApacheSystemUtils |
20-
f.getName() = "AWT_TOOLKIT" and propertyName = "awt.toolkit"
55+
f.hasName("AWT_TOOLKIT") and propertyName = "awt.toolkit"
2156
or
22-
f.getName() = "FILE_ENCODING" and propertyName = "file.encoding"
57+
f.hasName("FILE_ENCODING") and propertyName = "file.encoding"
2358
or
24-
f.getName() = "FILE_SEPARATOR" and propertyName = "file.separator"
59+
f.hasName("FILE_SEPARATOR") and propertyName = "file.separator"
2560
or
26-
f.getName() = "JAVA_AWT_FONTS" and propertyName = "java.awt.fonts"
61+
f.hasName("JAVA_AWT_FONTS") and propertyName = "java.awt.fonts"
2762
or
28-
f.getName() = "JAVA_AWT_GRAPHICSENV" and propertyName = "java.awt.graphicsenv"
63+
f.hasName("JAVA_AWT_GRAPHICSENV") and propertyName = "java.awt.graphicsenv"
2964
or
30-
f.getName() = "JAVA_AWT_HEADLESS" and propertyName = "java.awt.headless"
65+
f.hasName("JAVA_AWT_HEADLESS") and propertyName = "java.awt.headless"
3166
or
32-
f.getName() = "JAVA_AWT_PRINTERJOB" and propertyName = "java.awt.printerjob"
67+
f.hasName("JAVA_AWT_PRINTERJOB") and propertyName = "java.awt.printerjob"
3368
or
34-
f.getName() = "JAVA_CLASS_PATH" and propertyName = "java.class.path"
69+
f.hasName("JAVA_CLASS_PATH") and propertyName = "java.class.path"
3570
or
36-
f.getName() = "JAVA_CLASS_VERSION" and propertyName = "java.class.version"
71+
f.hasName("JAVA_CLASS_VERSION") and propertyName = "java.class.version"
3772
or
38-
f.getName() = "JAVA_COMPILER" and propertyName = "java.compiler"
73+
f.hasName("JAVA_COMPILER") and propertyName = "java.compiler"
3974
or
40-
f.getName() = "JAVA_EXT_DIRS" and propertyName = "java.ext.dirs"
75+
f.hasName("JAVA_EXT_DIRS") and propertyName = "java.ext.dirs"
4176
or
42-
f.getName() = "JAVA_HOME" and propertyName = "java.home"
77+
f.hasName("JAVA_HOME") and propertyName = "java.home"
4378
or
44-
f.getName() = "JAVA_IO_TMPDIR" and propertyName = "java.io.tmpdir"
79+
f.hasName("JAVA_IO_TMPDIR") and propertyName = "java.io.tmpdir"
4580
or
46-
f.getName() = "JAVA_LIBRARY_PATH" and propertyName = "java.library.path"
81+
f.hasName("JAVA_LIBRARY_PATH") and propertyName = "java.library.path"
4782
or
48-
f.getName() = "JAVA_RUNTIME_NAME" and propertyName = "java.runtime.name"
83+
f.hasName("JAVA_RUNTIME_NAME") and propertyName = "java.runtime.name"
4984
or
50-
f.getName() = "JAVA_RUNTIME_VERSION" and propertyName = "java.runtime.version"
85+
f.hasName("JAVA_RUNTIME_VERSION") and propertyName = "java.runtime.version"
5186
or
52-
f.getName() = "JAVA_SPECIFICATION_NAME" and propertyName = "java.specification.name"
87+
f.hasName("JAVA_SPECIFICATION_NAME") and propertyName = "java.specification.name"
5388
or
54-
f.getName() = "JAVA_SPECIFICATION_VENDOR" and propertyName = "java.specification.vendor"
89+
f.hasName("JAVA_SPECIFICATION_VENDOR") and propertyName = "java.specification.vendor"
5590
or
56-
f.getName() = "JAVA_UTIL_PREFS_PREFERENCES_FACTORY" and
91+
f.hasName("JAVA_UTIL_PREFS_PREFERENCES_FACTORY") and
5792
propertyName = "java.util.prefs.PreferencesFactory"
5893
or
59-
f.getName() = "JAVA_VENDOR" and propertyName = "java.vendor"
94+
f.hasName("JAVA_VENDOR") and propertyName = "java.vendor"
6095
or
61-
f.getName() = "JAVA_VENDOR_URL" and propertyName = "java.vendor.url"
96+
f.hasName("JAVA_VENDOR_URL") and propertyName = "java.vendor.url"
6297
or
63-
f.getName() = "JAVA_VERSION" and propertyName = "java.version"
98+
f.hasName("JAVA_VERSION") and propertyName = "java.version"
6499
or
65-
f.getName() = "JAVA_VM_INFO" and propertyName = "java.vm.info"
100+
f.hasName("JAVA_VM_INFO") and propertyName = "java.vm.info"
66101
or
67-
f.getName() = "JAVA_VM_NAME" and propertyName = "java.vm.name"
102+
f.hasName("JAVA_VM_NAME") and propertyName = "java.vm.name"
68103
or
69-
f.getName() = "JAVA_VM_SPECIFICATION_NAME" and propertyName = "java.vm.specification.name"
104+
f.hasName("JAVA_VM_SPECIFICATION_NAME") and propertyName = "java.vm.specification.name"
70105
or
71-
f.getName() = "JAVA_VM_SPECIFICATION_VENDOR" and propertyName = "java.vm.specification.vendor"
106+
f.hasName("JAVA_VM_SPECIFICATION_VENDOR") and propertyName = "java.vm.specification.vendor"
72107
or
73-
f.getName() = "JAVA_VM_VENDOR" and propertyName = "java.vm.vendor"
108+
f.hasName("JAVA_VM_VENDOR") and propertyName = "java.vm.vendor"
74109
or
75-
f.getName() = "JAVA_VM_VERSION" and propertyName = "java.vm.version"
110+
f.hasName("JAVA_VM_VERSION") and propertyName = "java.vm.version"
76111
or
77-
f.getName() = "LINE_SEPARATOR" and propertyName = "line.separator"
112+
f.hasName("LINE_SEPARATOR") and propertyName = "line.separator"
78113
or
79-
f.getName() = "OS_ARCH" and propertyName = "os.arch"
114+
f.hasName("OS_ARCH") and propertyName = "os.arch"
80115
or
81-
f.getName() = "OS_NAME" and propertyName = "os.name"
116+
f.hasName("OS_NAME") and propertyName = "os.name"
82117
or
83-
f.getName() = "OS_VERSION" and propertyName = "os.version"
118+
f.hasName("OS_VERSION") and propertyName = "os.version"
84119
or
85-
f.getName() = "PATH_SEPARATOR" and propertyName = "path.separator"
120+
f.hasName("PATH_SEPARATOR") and propertyName = "path.separator"
86121
or
87-
f.getName() = "USER_COUNTRY" and propertyName = "user.country"
122+
f.hasName("USER_COUNTRY") and propertyName = "user.country"
88123
or
89-
f.getName() = "USER_DIR" and propertyName = "user.dir"
124+
f.hasName("USER_DIR") and propertyName = "user.dir"
90125
or
91-
f.getName() = "USER_HOME" and propertyName = "user.home"
126+
f.hasName("USER_HOME") and propertyName = "user.home"
92127
or
93-
f.getName() = "USER_LANGUAGE" and propertyName = "user.language"
128+
f.hasName("USER_LANGUAGE") and propertyName = "user.language"
94129
or
95-
f.getName() = "USER_NAME" and propertyName = "user.name"
130+
f.hasName("USER_NAME") and propertyName = "user.name"
96131
or
97-
f.getName() = "USER_TIMEZONE" and propertyName = "user.timezone"
132+
f.hasName("USER_TIMEZONE") and propertyName = "user.timezone"
98133
)
99134
}
100135

@@ -109,6 +144,70 @@ private MethodAccess getSystemPropertyFromApacheFileUtils(string propertyName) {
109144
)
110145
}
111146

147+
private MethodAccess getSystemPropertyFromGuava(string propertyName) {
148+
exists(EnumConstant ec |
149+
ec.getDeclaringType().hasQualifiedName("com.google.common.base", "StandardSystemProperty") and
150+
result.getQualifier() = ec.getAnAccess() and
151+
result.getMethod().hasName("value")
152+
|
153+
ec.hasName("JAVA_VERSION") and propertyName = "java.version"
154+
or
155+
ec.hasName("JAVA_VENDOR") and propertyName = "java.vendor"
156+
or
157+
ec.hasName("JAVA_VENDOR_URL") and propertyName = "java.vendor.url"
158+
or
159+
ec.hasName("JAVA_HOME") and propertyName = "java.home"
160+
or
161+
ec.hasName("JAVA_VM_SPECIFICATION_VERSION") and propertyName = "java.vm.specification.version"
162+
or
163+
ec.hasName("JAVA_VM_SPECIFICATION_VENDOR") and propertyName = "java.vm.specification.vendor"
164+
or
165+
ec.hasName("JAVA_VM_SPECIFICATION_NAME") and propertyName = "java.vm.specification.name"
166+
or
167+
ec.hasName("JAVA_VM_VERSION") and propertyName = "java.vm.version"
168+
or
169+
ec.hasName("JAVA_VM_VENDOR") and propertyName = "java.vm.vendor"
170+
or
171+
ec.hasName("JAVA_VM_NAME") and propertyName = "java.vm.name"
172+
or
173+
ec.hasName("JAVA_SPECIFICATION_VERSION") and propertyName = "java.specification.version"
174+
or
175+
ec.hasName("JAVA_SPECIFICATION_VENDOR") and propertyName = "java.specification.vendor"
176+
or
177+
ec.hasName("JAVA_SPECIFICATION_NAME") and propertyName = "java.specification.name"
178+
or
179+
ec.hasName("JAVA_CLASS_VERSION") and propertyName = "java.class.version"
180+
or
181+
ec.hasName("JAVA_CLASS_PATH") and propertyName = "java.class.path"
182+
or
183+
ec.hasName("JAVA_LIBRARY_PATH") and propertyName = "java.library.path"
184+
or
185+
ec.hasName("JAVA_IO_TMPDIR") and propertyName = "java.io.tmpdir"
186+
or
187+
ec.hasName("JAVA_COMPILER") and propertyName = "java.compiler"
188+
or
189+
ec.hasName("JAVA_EXT_DIRS") and propertyName = "java.ext.dirs"
190+
or
191+
ec.hasName("OS_NAME") and propertyName = "os.name"
192+
or
193+
ec.hasName("OS_ARCH") and propertyName = "os.arch"
194+
or
195+
ec.hasName("OS_VERSION") and propertyName = "os.version"
196+
or
197+
ec.hasName("FILE_SEPARATOR") and propertyName = "file.separator"
198+
or
199+
ec.hasName("PATH_SEPARATOR") and propertyName = "path.separator"
200+
or
201+
ec.hasName("LINE_SEPARATOR") and propertyName = "line.separator"
202+
or
203+
ec.hasName("USER_NAME") and propertyName = "user.name"
204+
or
205+
ec.hasName("USER_HOME") and propertyName = "user.home"
206+
or
207+
ec.hasName("USER_DIR") and propertyName = "user.dir"
208+
)
209+
}
210+
112211
private MethodAccess getSystemPropertyFromOperatingSystemMXBean(string propertyName) {
113212
exists(Method m |
114213
m = result.getMethod() and
@@ -121,3 +220,12 @@ private MethodAccess getSystemPropertyFromOperatingSystemMXBean(string propertyN
121220
m.getName() = "getVersion" and propertyName = "os.version"
122221
)
123222
}
223+
224+
private MethodAccess getSystemPropertyFromSpringProperties(string propertyName) {
225+
exists(Method m |
226+
m = result.getMethod() and
227+
m.getDeclaringType().hasQualifiedName("org.springframework.core", "SpringProperties") and
228+
m.hasName("getProperty")
229+
) and
230+
result.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName
231+
}

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,39 +68,32 @@ private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAc
6868
}
6969

7070
/**
71-
* Holds when the Guard is an equality check between the Field `f` and the string or char constant `compareToLiteral`.
71+
* Holds when the Guard is an equality check between the system property with the name `propertyName`
72+
* and the string or char constant `compareToLiteral`.
7273
*/
73-
private Guard isOsFromFieldEqualityCheck(Field f, string compareToLiteral) {
74+
private Guard isOsFromFieldEqualityCheck(string propertyName, string compareToLiteral) {
7475
result
75-
.isEquality(any(FieldAccess fa | fa.getField() = f),
76+
.isEquality(getSystemProperty(propertyName),
7677
any(Literal literal |
7778
(literal instanceof CharacterLiteral or literal instanceof StringLiteral) and
7879
literal.getValue() = compareToLiteral
7980
), _)
8081
}
8182

8283
private class IsWindowsFromCharPathSeperator extends IsWindowsGuard {
83-
IsWindowsFromCharPathSeperator() {
84-
this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ";")
85-
}
84+
IsWindowsFromCharPathSeperator() { this = isOsFromFieldEqualityCheck("path.separator", "\\") }
8685
}
8786

8887
private class IsWindowsFromCharSeperator extends IsWindowsGuard {
89-
IsWindowsFromCharSeperator() {
90-
this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "\\")
91-
}
88+
IsWindowsFromCharSeperator() { this = isOsFromFieldEqualityCheck("file.separator", ";") }
9289
}
9390

9491
private class IsUnixFromCharPathSeperator extends IsUnixGuard {
95-
IsUnixFromCharPathSeperator() {
96-
this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ":")
97-
}
92+
IsUnixFromCharPathSeperator() { this = isOsFromFieldEqualityCheck("path.separator", "/") }
9893
}
9994

10095
private class IsUnixFromCharSeperator extends IsUnixGuard {
101-
IsUnixFromCharSeperator() {
102-
this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "/")
103-
}
96+
IsUnixFromCharSeperator() { this = isOsFromFieldEqualityCheck("file.separator", ":") }
10497
}
10598

10699
private class IsUnixFromSystemProp extends IsAnyUnixGuard instanceof MethodAccess {

java/ql/test/library-tests/os/Test.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ void testWindows() {
5656
if (File.separator == "\\") {
5757
onlyOnWindows();
5858
}
59+
60+
if (System.getProperty("path.separator").equals("\\")) {
61+
onlyOnWindows();
62+
}
5963
}
6064

6165
void testUnix() {
@@ -89,6 +93,10 @@ void testUnix() {
8993
if (File.separator == "/") {
9094
onlyOnUnix();
9195
}
96+
97+
if (System.getProperty("path.separator").equals("/")) {
98+
onlyOnUnix();
99+
}
92100
}
93101

94102
void testLinux() {

0 commit comments

Comments
 (0)