diff --git a/java/ql/lib/semmle/code/java/JDK.qll b/java/ql/lib/semmle/code/java/JDK.qll index 504c70bff86..0a5153f7042 100644 --- a/java/ql/lib/semmle/code/java/JDK.qll +++ b/java/ql/lib/semmle/code/java/JDK.qll @@ -37,6 +37,27 @@ class StringLengthMethod extends Method { StringLengthMethod() { this.hasName("length") and this.getDeclaringType() instanceof TypeString } } +/** + * The methods on the class `java.lang.String` that are used to perform partial matches with a specified substring or char. + */ +class StringPartialMatchMethod extends Method { + StringPartialMatchMethod() { + this.hasName([ + "contains", "startsWith", "endsWith", "matches", "indexOf", "lastIndexOf", "regionMatches" + ]) and + this.getDeclaringType() instanceof TypeString + } + + /** + * The index of the parameter that is being matched against. + */ + int getMatchParameterIndex() { + if not this.hasName("regionMatches") + then result = 0 + else this.getParameterType(result) instanceof TypeString + } +} + /** The class `java.lang.StringBuffer`. */ class TypeStringBuffer extends Class { TypeStringBuffer() { this.hasQualifiedName("java.lang", "StringBuffer") } diff --git a/java/ql/lib/semmle/code/java/StringCheck.qll b/java/ql/lib/semmle/code/java/StringCheck.qll deleted file mode 100644 index eaf15fdfa29..00000000000 --- a/java/ql/lib/semmle/code/java/StringCheck.qll +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Provides classes and predicates for reasoning about checking characterizations about strings. - */ - -import java - -/** - * Holds if `ma` is a call to a method that checks a partial string match. - */ -predicate isStringPartialMatch(MethodAccess ma) { - ma.getMethod().getDeclaringType() instanceof TypeString and - ma.getMethod().getName() = - ["contains", "startsWith", "matches", "regionMatches", "indexOf", "lastIndexOf"] - } diff --git a/java/ql/lib/semmle/code/java/os/OSCheck.qll b/java/ql/lib/semmle/code/java/os/OSCheck.qll index aa250947583..587ed49a70b 100644 --- a/java/ql/lib/semmle/code/java/os/OSCheck.qll +++ b/java/ql/lib/semmle/code/java/os/OSCheck.qll @@ -6,7 +6,6 @@ import java import semmle.code.java.controlflow.Guards private import semmle.code.java.frameworks.apache.Lang private import semmle.code.java.dataflow.DataFlow -private import semmle.code.java.StringCheck /** * A guard that checks if the current platform is Windows. @@ -21,34 +20,36 @@ abstract class IsUnixGuard extends Guard { } /** * Holds when the MethodAccess is a call to check the current OS using either the upper case `osUpperCase` or lower case `osLowerCase` string constants. */ -bindingset[osUpperCase, osLowerCase] -private predicate isOsFromSystemProp(MethodAccess ma, string osUpperCase, string osLowerCase) { - exists(MethodAccessSystemGetProperty sgpMa | +bindingset[osString] +private predicate isOsFromSystemProp(MethodAccess ma, string osString) { + exists(MethodAccessSystemGetProperty sgpMa, Expr sgpFlowsToExpr | sgpMa.hasCompileTimeConstantGetPropertyName("os.name") | - DataFlow::localExprFlow(sgpMa, ma.getQualifier()) and // Call from System.getProperty to some partial match method - ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().matches(osUpperCase) - or - exists(MethodAccess toLowerCaseMa | - toLowerCaseMa.getMethod() = - any(Method m | m.getDeclaringType() instanceof TypeString and m.hasName("toLowerCase")) - | - DataFlow::localExprFlow(sgpMa, toLowerCaseMa.getQualifier()) and // Call from System.getProperty to toLowerCase - DataFlow::localExprFlow(toLowerCaseMa, ma.getQualifier()) and // Call from toLowerCase to some partial match method - ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().matches(osLowerCase) + DataFlow::localExprFlow(sgpMa, sgpFlowsToExpr) and + ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().toLowerCase().matches(osString) and // Call from System.getProperty to some partial match method + ( + sgpFlowsToExpr = 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 + DataFlow::localExprFlow(caseChangeMa, ma.getQualifier()) // Call from case-switching method to some partial match method + ) ) ) and - isStringPartialMatch(ma) + ma.getMethod() instanceof StringPartialMatchMethod } private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAccess { - IsWindowsFromSystemProp() { isOsFromSystemProp(this, "Window%", "window%") } + IsWindowsFromSystemProp() { isOsFromSystemProp(this, "window%") } } private class IsUnixFromSystemProp extends IsUnixGuard instanceof MethodAccess { - IsUnixFromSystemProp() { - isOsFromSystemProp(this, ["Mac%", "Linux%", "LINUX%"], ["mac%", "linux%"]) - } + IsUnixFromSystemProp() { isOsFromSystemProp(this, ["mac%", "linux%"]) } } bindingset[fieldNamePattern] @@ -70,7 +71,7 @@ private class IsUnixFromApacheCommons extends IsUnixGuard instanceof FieldAccess /** * A guard that checks if the `java.nio.file.FileSystem` supports posix file permissions. * This is often used to infer if the OS is unix-based. - * Looks for calls to `contains("posix")` on the `supportedFileAttributeViews` method returned by `FileSystem`. + * Looks for calls to `contains("posix")` on the `supportedFileAttributeViews()` method returned by `FileSystem`. */ private class IsUnixFromPosixFromFileSystem extends IsUnixGuard instanceof MethodAccess { IsUnixFromPosixFromFileSystem() { diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql index 6582a446f4f..3839ad6934d 100644 --- a/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql +++ b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql @@ -104,17 +104,17 @@ private class FileCreateTempFileSink extends FileCreationSink { } /** - * A guard that checks what OS the program is running on. + * A guard that checks whether or not the the program is running on the Windows OS. */ -abstract private class OsBarrierGuard extends DataFlow::BarrierGuard { } +abstract private class WindowsOsBarrierGuard extends DataFlow::BarrierGuard { } -private class IsUnixBarrierGuard extends OsBarrierGuard instanceof IsUnixGuard { +private class IsNotUnixBarrierGuard extends WindowsOsBarrierGuard instanceof IsUnixGuard { override predicate checks(Expr e, boolean branch) { this.controls(e.getBasicBlock(), branch.booleanNot()) } } -private class IsWindowsBarrierGuard extends OsBarrierGuard instanceof IsWindowsGuard { +private class IsWindowsBarrierGuard extends WindowsOsBarrierGuard instanceof IsWindowsGuard { override predicate checks(Expr e, boolean branch) { this.controls(e.getBasicBlock(), branch) } } @@ -147,7 +147,7 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf } override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { - guard instanceof OsBarrierGuard + guard instanceof WindowsOsBarrierGuard } } diff --git a/java/ql/test/library-tests/JDK/PrintAst.expected b/java/ql/test/library-tests/JDK/PrintAst.expected index 6aff48adce5..e6f240b325e 100644 --- a/java/ql/test/library-tests/JDK/PrintAst.expected +++ b/java/ql/test/library-tests/JDK/PrintAst.expected @@ -60,6 +60,75 @@ jdk/A.java: # 28| 0: [ArrayTypeAccess] ...[] # 28| 0: [TypeAccess] String # 28| 5: [BlockStmt] { ... } +jdk/StringMatch.java: +# 0| [CompilationUnit] StringMatch +# 1| 1: [Class] StringMatch +# 2| 3: [FieldDeclaration] String STR; +# 2| -1: [TypeAccess] String +# 2| 0: [StringLiteral] "the quick brown fox jumps over the lazy dog" +# 4| 4: [Method] a +# 4| 3: [TypeAccess] void +# 4| 5: [BlockStmt] { ... } +# 5| 0: [ExprStmt] ; +# 5| 0: [MethodAccess] matches(...) +# 5| -1: [VarAccess] STR +# 5| 0: [StringLiteral] "[a-z]+" +# 8| 5: [Method] b +# 8| 3: [TypeAccess] void +# 8| 5: [BlockStmt] { ... } +# 9| 0: [ExprStmt] ; +# 9| 0: [MethodAccess] contains(...) +# 9| -1: [VarAccess] STR +# 9| 0: [StringLiteral] "the" +# 12| 6: [Method] c +# 12| 3: [TypeAccess] void +# 12| 5: [BlockStmt] { ... } +# 13| 0: [ExprStmt] ; +# 13| 0: [MethodAccess] startsWith(...) +# 13| -1: [VarAccess] STR +# 13| 0: [StringLiteral] "the" +# 16| 7: [Method] d +# 16| 3: [TypeAccess] void +# 16| 5: [BlockStmt] { ... } +# 17| 0: [ExprStmt] ; +# 17| 0: [MethodAccess] endsWith(...) +# 17| -1: [VarAccess] STR +# 17| 0: [StringLiteral] "dog" +# 20| 8: [Method] e +# 20| 3: [TypeAccess] void +# 20| 5: [BlockStmt] { ... } +# 21| 0: [ExprStmt] ; +# 21| 0: [MethodAccess] indexOf(...) +# 21| -1: [VarAccess] STR +# 21| 0: [StringLiteral] "lazy" +# 24| 9: [Method] f +# 24| 3: [TypeAccess] void +# 24| 5: [BlockStmt] { ... } +# 25| 0: [ExprStmt] ; +# 25| 0: [MethodAccess] lastIndexOf(...) +# 25| -1: [VarAccess] STR +# 25| 0: [StringLiteral] "lazy" +# 28| 10: [Method] g +# 28| 3: [TypeAccess] void +# 28| 5: [BlockStmt] { ... } +# 29| 0: [ExprStmt] ; +# 29| 0: [MethodAccess] regionMatches(...) +# 29| -1: [VarAccess] STR +# 29| 0: [IntegerLiteral] 0 +# 29| 1: [StringLiteral] "fox" +# 29| 2: [IntegerLiteral] 0 +# 29| 3: [IntegerLiteral] 4 +# 32| 11: [Method] h +# 32| 3: [TypeAccess] void +# 32| 5: [BlockStmt] { ... } +# 33| 0: [ExprStmt] ; +# 33| 0: [MethodAccess] regionMatches(...) +# 33| -1: [VarAccess] STR +# 33| 0: [BooleanLiteral] true +# 33| 1: [IntegerLiteral] 0 +# 33| 2: [StringLiteral] "FOX" +# 33| 3: [IntegerLiteral] 0 +# 33| 4: [IntegerLiteral] 4 jdk/SystemGetPropertyCall.java: # 0| [CompilationUnit] SystemGetPropertyCall # 3| 1: [Class] SystemGetPropertyCall diff --git a/java/ql/test/library-tests/JDK/StringMatch.expected b/java/ql/test/library-tests/JDK/StringMatch.expected new file mode 100644 index 00000000000..ae88868ae82 --- /dev/null +++ b/java/ql/test/library-tests/JDK/StringMatch.expected @@ -0,0 +1,8 @@ +| jdk/StringMatch.java:5:9:5:29 | matches(...) | jdk/StringMatch.java:5:21:5:28 | "[a-z]+" | +| jdk/StringMatch.java:9:9:9:27 | contains(...) | jdk/StringMatch.java:9:22:9:26 | "the" | +| jdk/StringMatch.java:13:9:13:29 | startsWith(...) | jdk/StringMatch.java:13:24:13:28 | "the" | +| jdk/StringMatch.java:17:9:17:27 | endsWith(...) | jdk/StringMatch.java:17:22:17:26 | "dog" | +| jdk/StringMatch.java:21:9:21:27 | indexOf(...) | jdk/StringMatch.java:21:21:21:26 | "lazy" | +| jdk/StringMatch.java:25:9:25:31 | lastIndexOf(...) | jdk/StringMatch.java:25:25:25:30 | "lazy" | +| jdk/StringMatch.java:29:9:29:41 | regionMatches(...) | jdk/StringMatch.java:29:30:29:34 | "fox" | +| jdk/StringMatch.java:33:9:33:47 | regionMatches(...) | jdk/StringMatch.java:33:36:33:40 | "FOX" | diff --git a/java/ql/test/library-tests/JDK/StringMatch.ql b/java/ql/test/library-tests/JDK/StringMatch.ql new file mode 100644 index 00000000000..213c72a71e8 --- /dev/null +++ b/java/ql/test/library-tests/JDK/StringMatch.ql @@ -0,0 +1,5 @@ +import java + +from MethodAccess ma, StringPartialMatchMethod m +where ma.getMethod() = m +select ma, ma.getArgument(m.getMatchParameterIndex()) diff --git a/java/ql/test/library-tests/JDK/jdk/StringMatch.java b/java/ql/test/library-tests/JDK/jdk/StringMatch.java new file mode 100644 index 00000000000..3ceb1a06b5d --- /dev/null +++ b/java/ql/test/library-tests/JDK/jdk/StringMatch.java @@ -0,0 +1,35 @@ +public class StringMatch { + private static String STR = "the quick brown fox jumps over the lazy dog"; + + void a() { + STR.matches("[a-z]+"); + } + + void b() { + STR.contains("the"); + } + + void c() { + STR.startsWith("the"); + } + + void d() { + STR.endsWith("dog"); + } + + void e() { + STR.indexOf("lazy"); + } + + void f() { + STR.lastIndexOf("lazy"); + } + + void g() { + STR.regionMatches(0, "fox", 0, 4); + } + + void h() { + STR.regionMatches(true, 0, "FOX", 0, 4); + } +}