Skip to content

Commit 3c53a05

Browse files
committed
Add OS Checks based upon separator or path separator
1 parent 82d3cd8 commit 3c53a05

File tree

8 files changed

+103
-21
lines changed

8 files changed

+103
-21
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,21 @@ 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+
205220
// --- Standard methods ---
206221
/**
207222
* Any constructor of class `java.lang.ProcessBuilder`.

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,13 +325,10 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
325325
or
326326
exists(Field f |
327327
e = f.getAnAccess() and
328-
f.getDeclaringType() instanceof TypeFile and
329328
fmtvalue = "x" // dummy value
330329
|
331-
f.hasName("pathSeparator") or
332-
f.hasName("pathSeparatorChar") or
333-
f.hasName("separator") or
334-
f.hasName("separatorChar")
330+
f instanceof FieldFileSeparator or
331+
f instanceof FieldFilePathSeperator
335332
)
336333
)
337334
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,42 @@ private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAc
6767
IsWindowsFromSystemProp() { isOsFromSystemProp(this, "window%") }
6868
}
6969

70+
/**
71+
* Holds when the Guard is an equality check between the Field `f` and the string or char constant `compareToLiteral`.
72+
*/
73+
private Guard isOsFromFieldEqualityCheck(Field f, string compareToLiteral) {
74+
result
75+
.isEquality(any(FieldAccess fa | fa.getField() = f),
76+
any(Literal literal |
77+
(literal instanceof CharacterLiteral or literal instanceof StringLiteral) and
78+
literal.getValue() = compareToLiteral
79+
), _)
80+
}
81+
82+
private class IsWindowsFromCharPathSeperator extends IsWindowsGuard {
83+
IsWindowsFromCharPathSeperator() {
84+
this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ";")
85+
}
86+
}
87+
88+
private class IsWindowsFromCharSeperator extends IsWindowsGuard {
89+
IsWindowsFromCharSeperator() {
90+
this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "\\")
91+
}
92+
}
93+
94+
private class IsUnixFromCharPathSeperator extends IsUnixGuard {
95+
IsUnixFromCharPathSeperator() {
96+
this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ":")
97+
}
98+
}
99+
100+
private class IsUnixFromCharSeperator extends IsUnixGuard {
101+
IsUnixFromCharSeperator() {
102+
this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "/")
103+
}
104+
}
105+
70106
private class IsUnixFromSystemProp extends IsAnyUnixGuard instanceof MethodAccess {
71107
IsUnixFromSystemProp() { isOsFromSystemProp(this, ["mac%", "linux%"]) }
72108
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
2+
import java.io.File;
13
import java.nio.file.FileSystems;
24
import java.nio.file.Path;
35

@@ -38,6 +40,22 @@ void testWindows() {
3840
} else {
3941
// Might be another version of windows
4042
}
43+
44+
if (File.pathSeparatorChar == ';') {
45+
onlyOnWindows();
46+
}
47+
48+
if (File.pathSeparator == ";") {
49+
onlyOnWindows();
50+
}
51+
52+
if (File.separatorChar == '\\') {
53+
onlyOnWindows();
54+
}
55+
56+
if (File.separator == "\\") {
57+
onlyOnWindows();
58+
}
4159
}
4260

4361
void testUnix() {
@@ -55,6 +73,22 @@ void testUnix() {
5573
// Reasonable assumption, maybe not 100% accurate, but it's 'good enough'
5674
onlyOnWindows();
5775
}
76+
77+
if (File.pathSeparatorChar == ':') {
78+
onlyOnUnix();
79+
}
80+
81+
if (File.pathSeparator == ":") {
82+
onlyOnUnix();
83+
}
84+
85+
if (File.separatorChar == '/') {
86+
onlyOnUnix();
87+
}
88+
89+
if (File.separator == "/") {
90+
onlyOnUnix();
91+
}
5892
}
5993

6094
void testLinux() {

java/ql/test/library-tests/os/any-unix-test.expected

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1415:5:1415:84 | IS_OS_SOLARIS |
2828
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1427:5:1427:83 | IS_OS_SUN_OS |
2929
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1625:5:1625:80 | IS_OS_ZOS |
30-
| Test.java:61:13:61:73 | contains(...) |
31-
| Test.java:65:13:65:59 | contains(...) |
32-
| Test.java:69:13:69:35 | SystemUtils.IS_OS_LINUX |
33-
| Test.java:75:14:75:36 | SystemUtils.IS_OS_LINUX |
34-
| Test.java:83:13:83:62 | contains(...) |
35-
| Test.java:87:14:87:72 | contains(...) |
36-
| Test.java:91:14:91:34 | SystemUtils.IS_OS_MAC |
37-
| Test.java:97:14:97:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |
30+
| Test.java:95:13:95:73 | contains(...) |
31+
| Test.java:99:13:99:59 | contains(...) |
32+
| Test.java:103:13:103:35 | SystemUtils.IS_OS_LINUX |
33+
| Test.java:109:14:109:36 | SystemUtils.IS_OS_LINUX |
34+
| Test.java:117:13:117:62 | contains(...) |
35+
| Test.java:121:14:121:72 | contains(...) |
36+
| Test.java:125:14:125:34 | SystemUtils.IS_OS_MAC |
37+
| Test.java:131:14:131:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |

java/ql/test/library-tests/os/any-windows-test.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@
1111
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1584:5:1584:86 | IS_OS_WINDOWS_7 |
1212
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1596:5:1596:86 | IS_OS_WINDOWS_8 |
1313
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1608:5:1608:87 | IS_OS_WINDOWS_10 |
14-
| Test.java:36:13:36:40 | SystemUtils.IS_OS_WINDOWS_XP |
14+
| Test.java:38:13:38:40 | SystemUtils.IS_OS_WINDOWS_XP |
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1439:5:1439:81 | IS_OS_UNIX |
2-
| Test.java:44:13:44:95 | contains(...) |
3-
| Test.java:48:13:48:84 | contains(...) |
4-
| Test.java:52:13:52:34 | SystemUtils.IS_OS_UNIX |
2+
| Test.java:62:13:62:95 | contains(...) |
3+
| Test.java:66:13:66:84 | contains(...) |
4+
| Test.java:70:13:70:34 | SystemUtils.IS_OS_UNIX |
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
| ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1451:5:1451:84 | IS_OS_WINDOWS |
2-
| Test.java:18:13:18:61 | contains(...) |
3-
| Test.java:22:13:22:75 | contains(...) |
4-
| Test.java:26:13:26:75 | contains(...) |
5-
| Test.java:30:13:30:37 | SystemUtils.IS_OS_WINDOWS |
2+
| Test.java:20:13:20:61 | contains(...) |
3+
| Test.java:24:13:24:75 | contains(...) |
4+
| Test.java:28:13:28:75 | contains(...) |
5+
| Test.java:32:13:32:37 | SystemUtils.IS_OS_WINDOWS |

0 commit comments

Comments
 (0)