Java: Enable BarrierGuard wrappers #20183
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables the Guards library wrapper guard integration with BarrierGuard in Java, allowing data flow sanitizers to leverage the full power of the Guards library including wrapped validation checks. This eliminates the need for ad-hoc support in individual barriers.
- Adds support for SSA definition-based guard checking in the Guards library
- Refactors BarrierGuard implementation to use wrapped validation checks with state
- Removes custom validation method logic from path sanitizers and arithmetic guards
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/Guards.qll | Adds guardChecksDef predicate for SSA definition validation and removes deprecated boolean guard logic |
| java/ql/lib/semmle/code/java/security/PathSanitizer.qll | Removes ValidationMethod module and simplifies sanitizer classes to use only BarrierGuard |
| java/ql/lib/semmle/code/java/security/ArithmeticCommon.qll | Refactors size guard predicates to work with expressions and updates guard checking to use BarrierGuard |
| java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll | Updates BarrierGuard implementation to use wrapped validation with SSA definitions |
| java/ql/lib/semmle/code/java/controlflow/Guards.qll | Removes deprecated implies_v3 predicate |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Fixed the kotlin qltest in two ways (either of the two added commits suffice to fix the test). Firstly, kotlin Throws annotations are now recognized - we definitely want that, as that's similar to the java |
aschackmull
force-pushed
the
java/barrierguard-wrappers
branch
from
1acee9d to
492a5ca
Compare
| private predicate methodMayThrow(Method m, ThrowableType t) { | ||
| exists(AstNode n | | ||
| t = n.(ThrowStmt).getThrownExceptionType() and | ||
| not n.(ThrowStmt).getParent() = any(Method m0).getBody() |
There was a problem hiding this comment.
Is this to filter out methods that always throw?
| @@ -210,7 +178,6 @@ predicate overflowSink(ArithExpr exp, VarAccess use) { | |||
| exp instanceof PostIncExpr or | |||
| exp instanceof MulExpr | |||
| ) and | |||
| not guardedLessThanSomething(use) and | |||
There was a problem hiding this comment.
Why is this removed (and the line below)?
There was a problem hiding this comment.
It's part of the barrier, so it's superfluous to filter like this in the sink definition.
aschackmull
added
the
no-change-note-required
This enables the Guards library wrapper guard integration with BarrierGuard such that data flow sanitizers make use of the full power of the Guards library including wrapped validation checks.
Some individual barriers had ad-hoc support for this, which can now be deleted as it becomes superfluous.