Skip to content

Commit c33e03d

Browse files
Jami CogswellJami Cogswell
Jami Cogswell
authored and
Jami Cogswell
committed
Java: initial sanitizer
1 parent 74ea974 commit c33e03d

File tree

1 file changed

+70
-0
lines changed

1 file changed

+70
-0
lines changed

Diff for: java/ql/lib/semmle/code/java/security/PathSanitizer.qll

+70
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,73 @@ private class FileGetNameSanitizer extends PathInjectionSanitizer {
352352
)
353353
}
354354
}
355+
356+
/**
357+
* A complementary sanitizer that protects against path injection vulnerabilities
358+
* by replacing all directory characters ('..', '/', and '\') with safe characters.
359+
*/
360+
private class ReplaceDirectoryCharactersSanitizer extends MethodCall {
361+
ReplaceDirectoryCharactersSanitizer() {
362+
exists(MethodCall mc |
363+
// TODO: "java.lang.String.replace" as well
364+
mc.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
365+
// TODO: unhardcode all of the below to handle more valid replacements and several calls
366+
(
367+
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\.\\.|[/\\\\]"
368+
or
369+
exists(MethodCall mc2 |
370+
mc2.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
371+
mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\." and
372+
mc2.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "/"
373+
)
374+
) and
375+
// TODO: accept more replacement characters?
376+
mc.getArgument(1).(CompileTimeConstantExpr).getStringValue() = ["", "_"] and
377+
this = mc
378+
)
379+
}
380+
}
381+
382+
/**
383+
* A complementary guard that protects against path traversal by looking
384+
* for patterns that exclude directory characters: `..`, '/', and '\'.
385+
*/
386+
private class DirectoryCharactersGuard extends PathGuard {
387+
Expr checkedExpr;
388+
389+
DirectoryCharactersGuard() {
390+
exists(MethodCall mc, Method m | m = mc.getMethod() |
391+
m.getDeclaringType() instanceof TypeString and
392+
m.hasName("matches") and
393+
// TODO: unhardcode to handle more valid matches
394+
mc.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "[0-9a-fA-F]{20,}" and
395+
checkedExpr = mc.getQualifier() and
396+
this = mc
397+
)
398+
}
399+
400+
override Expr getCheckedExpr() { result = checkedExpr }
401+
}
402+
403+
/**
404+
* Holds if `g` is a guard that considers a path safe because it is checked to make
405+
* sure it does not contain any directory characters: '..', '/', and '\'.
406+
*/
407+
private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) {
408+
branch = true and
409+
g instanceof DirectoryCharactersGuard and
410+
localTaintFlowToPathGuard(e, g)
411+
}
412+
413+
/**
414+
* A sanitizer that protects against path injection vulnerabilities
415+
* by ensuring that the path does not contain any directory characters:
416+
* '..', '/', and '\'.
417+
*/
418+
private class DirectoryCharactersSanitizer extends PathInjectionSanitizer {
419+
DirectoryCharactersSanitizer() {
420+
this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or
421+
this = DataFlow::BarrierGuard<directoryCharactersGuard/3>::getABarrierNode() or
422+
this = ValidationMethod<directoryCharactersGuard/3>::getAValidatedNode()
423+
}
424+
}

0 commit comments

Comments
 (0)