Add OS Checks based upon separator or path separator

2026-04-27 09:45:15 +02:00 · 2022-03-02 14:15:56 -05:00
parent 82d3cd8924
commit 3c53a05e16
8 changed files with 103 additions and 21 deletions
--- a/java/ql/lib/semmle/code/java/JDK.qll
+++ b/java/ql/lib/semmle/code/java/JDK.qll
@@ -202,6 +202,21 @@ class TypeFile extends Class {
  TypeFile() { this.hasQualifiedName("java.io", "File") }
 }

+/** The field `java.io.File.separator` or `java.io.File.separatorChar` */
+class FieldFileSeparator extends Field {
+  FieldFileSeparator() {
+    this.getDeclaringType() instanceof TypeFile and this.hasName(["separator", "separatorChar"])
+  }
+}
+
+/* The field `java.io.File.pathSeparator` or `java.io.File.pathSeparatorChar` */
+class FieldFilePathSeparator extends Field {
+  FieldFilePathSeparator() {
+    this.getDeclaringType() instanceof TypeFile and
+    this.hasName(["pathSeparator", "pathSeparatorChar"])
+  }
+}
+
 // --- Standard methods ---
 /**
 * Any constructor of class `java.lang.ProcessBuilder`.
--- a/java/ql/lib/semmle/code/java/StringFormat.qll
+++ b/java/ql/lib/semmle/code/java/StringFormat.qll
@@ -325,13 +325,10 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
    or
    exists(Field f |
      e = f.getAnAccess() and
-      f.getDeclaringType() instanceof TypeFile and
      fmtvalue = "x" // dummy value
    |
-      f.hasName("pathSeparator") or
-      f.hasName("pathSeparatorChar") or
-      f.hasName("separator") or
-      f.hasName("separatorChar")
+      f instanceof FieldFileSeparator or
+      f instanceof FieldFilePathSeperator
    )
  )
 }
--- a/java/ql/lib/semmle/code/java/os/OSCheck.qll
+++ b/java/ql/lib/semmle/code/java/os/OSCheck.qll
@@ -67,6 +67,42 @@ private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAc
  IsWindowsFromSystemProp() { isOsFromSystemProp(this, "window%") }
 }

+/**
+ * Holds when the Guard is an equality check between the Field `f` and the string or char constant `compareToLiteral`.
+ */
+private Guard isOsFromFieldEqualityCheck(Field f, string compareToLiteral) {
+  result
+      .isEquality(any(FieldAccess fa | fa.getField() = f),
+        any(Literal literal |
+          (literal instanceof CharacterLiteral or literal instanceof StringLiteral) and
+          literal.getValue() = compareToLiteral
+        ), _)
+}
+
+private class IsWindowsFromCharPathSeperator extends IsWindowsGuard {
+  IsWindowsFromCharPathSeperator() {
+    this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ";")
+  }
+}
+
+private class IsWindowsFromCharSeperator extends IsWindowsGuard {
+  IsWindowsFromCharSeperator() {
+    this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "\\")
+  }
+}
+
+private class IsUnixFromCharPathSeperator extends IsUnixGuard {
+  IsUnixFromCharPathSeperator() {
+    this = isOsFromFieldEqualityCheck(any(FieldFilePathSeparator f), ":")
+  }
+}
+
+private class IsUnixFromCharSeperator extends IsUnixGuard {
+  IsUnixFromCharSeperator() {
+    this = isOsFromFieldEqualityCheck(any(FieldFileSeparator f), "/")
+  }
+}
+
 private class IsUnixFromSystemProp extends IsAnyUnixGuard instanceof MethodAccess {
  IsUnixFromSystemProp() { isOsFromSystemProp(this, ["mac%", "linux%"]) }
 }
--- a/java/ql/test/library-tests/os/Test.java
+++ b/java/ql/test/library-tests/os/Test.java
@@ -1,3 +1,5 @@
+
+import java.io.File;
 import java.nio.file.FileSystems;
 import java.nio.file.Path;

@@ -38,6 +40,22 @@ public class Test {
        } else {
            // Might be another version of windows
        }
+
+        if (File.pathSeparatorChar == ';') {
+            onlyOnWindows();
+        }
+
+        if (File.pathSeparator == ";") {
+            onlyOnWindows();
+        }
+
+        if (File.separatorChar == '\\') {
+            onlyOnWindows();
+        }
+
+        if (File.separator == "\\") {
+            onlyOnWindows();
+        }
    }

    void testUnix() {
@@ -55,6 +73,22 @@ public class Test {
            // Reasonable assumption, maybe not 100% accurate, but it's 'good enough'
            onlyOnWindows();
        }
+
+        if (File.pathSeparatorChar == ':') {
+            onlyOnUnix();
+        }
+
+        if (File.pathSeparator == ":") {
+            onlyOnUnix();
+        }
+
+        if (File.separatorChar == '/') {
+            onlyOnUnix();
+        }
+
+        if (File.separator == "/") {
+            onlyOnUnix();
+        }
    }

    void testLinux() {
--- a/java/ql/test/library-tests/os/any-unix-test.expected
+++ b/java/ql/test/library-tests/os/any-unix-test.expected
@@ -27,11 +27,11 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1415:5:1415:84 | IS_OS_SOLARIS |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1427:5:1427:83 | IS_OS_SUN_OS |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1625:5:1625:80 | IS_OS_ZOS |
-| Test.java:61:13:61:73 | contains(...) |
-| Test.java:65:13:65:59 | contains(...) |
-| Test.java:69:13:69:35 | SystemUtils.IS_OS_LINUX |
-| Test.java:75:14:75:36 | SystemUtils.IS_OS_LINUX |
-| Test.java:83:13:83:62 | contains(...) |
-| Test.java:87:14:87:72 | contains(...) |
-| Test.java:91:14:91:34 | SystemUtils.IS_OS_MAC |
-| Test.java:97:14:97:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |
+| Test.java:95:13:95:73 | contains(...) |
+| Test.java:99:13:99:59 | contains(...) |
+| Test.java:103:13:103:35 | SystemUtils.IS_OS_LINUX |
+| Test.java:109:14:109:36 | SystemUtils.IS_OS_LINUX |
+| Test.java:117:13:117:62 | contains(...) |
+| Test.java:121:14:121:72 | contains(...) |
+| Test.java:125:14:125:34 | SystemUtils.IS_OS_MAC |
+| Test.java:131:14:131:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |
--- a/java/ql/test/library-tests/os/any-windows-test.expected
+++ b/java/ql/test/library-tests/os/any-windows-test.expected
@@ -11,4 +11,4 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1584:5:1584:86 | IS_OS_WINDOWS_7 |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1596:5:1596:86 | IS_OS_WINDOWS_8 |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1608:5:1608:87 | IS_OS_WINDOWS_10 |
-| Test.java:36:13:36:40 | SystemUtils.IS_OS_WINDOWS_XP |
+| Test.java:38:13:38:40 | SystemUtils.IS_OS_WINDOWS_XP |
--- a/java/ql/test/library-tests/os/unix-test.expected
+++ b/java/ql/test/library-tests/os/unix-test.expected
@@ -1,4 +1,4 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1439:5:1439:81 | IS_OS_UNIX |
-| Test.java:44:13:44:95 | contains(...) |
-| Test.java:48:13:48:84 | contains(...) |
-| Test.java:52:13:52:34 | SystemUtils.IS_OS_UNIX |
+| Test.java:62:13:62:95 | contains(...) |
+| Test.java:66:13:66:84 | contains(...) |
+| Test.java:70:13:70:34 | SystemUtils.IS_OS_UNIX |
--- a/java/ql/test/library-tests/os/windows-test.expected
+++ b/java/ql/test/library-tests/os/windows-test.expected
@@ -1,5 +1,5 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1451:5:1451:84 | IS_OS_WINDOWS |
-| Test.java:18:13:18:61 | contains(...) |
-| Test.java:22:13:22:75 | contains(...) |
-| Test.java:26:13:26:75 | contains(...) |
-| Test.java:30:13:30:37 | SystemUtils.IS_OS_WINDOWS |
+| Test.java:20:13:20:61 | contains(...) |
+| Test.java:24:13:24:75 | contains(...) |
+| Test.java:28:13:28:75 | contains(...) |
+| Test.java:32:13:32:37 | SystemUtils.IS_OS_WINDOWS |