Skip to content

Commit ab3690f

Browse files
Jami CogswellJami Cogswell
authored andcommitted
Java: initial sanitizer
1 parent 94080a6 commit ab3690f

File tree

1 file changed

+70
-0
lines changed

1 file changed

+70
-0
lines changed

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,3 +383,73 @@ private class FileConstructorChildArgumentStep extends AdditionalTaintStep {
383383
)
384384
}
385385
}
386+
387+
/**
388+
* A complementary sanitizer that protects against path injection vulnerabilities
389+
* by replacing all directory characters ('..', '/', and '\') with safe characters.
390+
*/
391+
private class ReplaceDirectoryCharactersSanitizer extends MethodCall {
392+
ReplaceDirectoryCharactersSanitizer() {
393+
exists(MethodCall mc |
394+
// TODO: "java.lang.String.replace" as well
395+
mc.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
396+
// TODO: unhardcode all of the below to handle more valid replacements and several calls
397+
(
398+
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\.\\.|[/\\\\]"
399+
or
400+
exists(MethodCall mc2 |
401+
mc2.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
402+
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\." and
403+
mc2.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "/"
404+
)
405+
) and
406+
// TODO: accept more replacement characters?
407+
mc.getArgument(1).(CompileTimeConstantExpr).getStringValue() = ["", "_"] and
408+
this = mc
409+
)
410+
}
411+
}
412+
413+
/**
414+
* A complementary guard that protects against path traversal by looking
415+
* for patterns that exclude directory characters: `..`, '/', and '\'.
416+
*/
417+
private class DirectoryCharactersGuard extends PathGuard {
418+
Expr checkedExpr;
419+
420+
DirectoryCharactersGuard() {
421+
exists(MethodCall mc, Method m | m = mc.getMethod() |
422+
m.getDeclaringType() instanceof TypeString and
423+
m.hasName("matches") and
424+
// TODO: unhardcode to handle more valid matches
425+
mc.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "[0-9a-fA-F]{20,}" and
426+
checkedExpr = mc.getQualifier() and
427+
this = mc
428+
)
429+
}
430+
431+
override Expr getCheckedExpr() { result = checkedExpr }
432+
}
433+
434+
/**
435+
* Holds if `g` is a guard that considers a path safe because it is checked to make
436+
* sure it does not contain any directory characters: '..', '/', and '\'.
437+
*/
438+
private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) {
439+
branch = true and
440+
g instanceof DirectoryCharactersGuard and
441+
localTaintFlowToPathGuard(e, g)
442+
}
443+
444+
/**
445+
* A sanitizer that protects against path injection vulnerabilities
446+
* by ensuring that the path does not contain any directory characters:
447+
* '..', '/', and '\'.
448+
*/
449+
private class DirectoryCharactersSanitizer extends PathInjectionSanitizer {
450+
DirectoryCharactersSanitizer() {
451+
this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or
452+
this = DataFlow::BarrierGuard<directoryCharactersGuard/3>::getABarrierNode() or
453+
this = ValidationMethod<directoryCharactersGuard/3>::getAValidatedNode()
454+
}
455+
}

0 commit comments

Comments
 (0)