Skip to content

Java: CWE-552 Query to detect unsafe request dispatcher usage #7286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jan 18, 2022
Merged

Java: CWE-552 Query to detect unsafe request dispatcher usage #7286

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8bcffc2
Query to detect unsafe request dispatcher usage
luchua-bc Dec 2, 2021
29ce0e9
Add sanitizer for virtual method calls
luchua-bc Dec 15, 2021
263dbd3
Optimize the query
luchua-bc Jan 12, 2022
cd9a485
Refactor NullOrEmptyCheckGuard
atorralba Jan 13, 2022
81feaae
Refactor PathMatchGuard
atorralba Jan 13, 2022
b6886b8
Move code to qll file
atorralba Jan 13, 2022
877c529
Remove the deprecated library keyword
luchua-bc Jan 14, 2022
136fefb
Apply suggestions from code review
atorralba Jan 14, 2022
fb1287d
Use dominance instead of getParent
atorralba Jan 14, 2022
eb1806c
Split PathMatchGuard into three guards
atorralba Jan 14, 2022
a2c98ba
Reordering
atorralba Jan 14, 2022
978ef15
Update method names
luchua-bc Jan 16, 2022
4797fce
Update use cases and qldoc
luchua-bc Jan 16, 2022
a3d65a8
Update recommendation in qldoc and make examples more comprehendible
luchua-bc Jan 18, 2022
748008a
Remove dangling reference to UnsafeRequestPath.java
smowton Jan 18, 2022
d744cf9
Clean up guard logic:
smowton Jan 18, 2022
1e32514
Avoid using `this` for a non-extending supertype, and remove needless…
smowton Jan 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use dominance instead of getParent 
Add clarification comments to PathMatchGuard
  • Loading branch information
@atorralba
atorralba committed Jan 14, 2022
commit fb1287d577d09cfd6710b51c466351e47fd624c4
24 changes: 18 additions & 6 deletions java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,31 @@ private class PathMatchGuard extends UnsafeUrlForwardBarrierGuard {
isExactStringPathMatch(this) and
branch = true
or
// When using an allowlist, that is, checking for known safe paths
// (for example, `path.startsWith(BASE_PATH)`)
// the application needs to protect against path traversal bypasses.
isAllowListCheck(this) and
exists(MethodAccess ma, Expr checked | isPathTraversalCheck(ma, checked) |
DataFlow::localExprFlow(checked, e)
or
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
// Either the path traversal check comes before the guard
DataFlow::localExprFlow(checked, e) or
// or both checks are in the same condition
// (for example, `path.startsWith(BASE_PATH) && !path.contains("..")`)
ma.(Guard).controls(this.getBasicBlock(), _) or
this.controls(ma.getBasicBlock(), branch)
) and
branch = true
or
// When using a blocklist, that is, checking for known bad patterns in the path,
// (for example, `path.startsWith("/WEB-INF/")` or `path.contains("..")`)
// the application needs to protect against double URL encoding bypasses.
isDisallowListCheck(this) and
exists(MethodAccess ma, Expr checked | isURLEncodingCheck(ma, checked) |
DataFlow::localExprFlow(checked, e)
or
ma.getParent*().(BinaryExpr) = this.(MethodAccess).getParent*()
// Either the URL encoding check comes before the guard
DataFlow::localExprFlow(checked, e) or
// or both checks are in the same condition
// (for example, `!path.contains("..") && !path.contains("%")`)
ma.(Guard).controls(this.getBasicBlock(), _) or
this.controls(ma.getBasicBlock(), branch)
) and
branch = false
)
Expand Down