Skip to content

Java: Prune PathGraph for CsrfUnprotectedRequestType.ql #20083

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
aschackmull merged 1 commit into from
Jul 18, 2025
+25 −2
Merged

Java: Prune PathGraph for CsrfUnprotectedRequestType.ql #20083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 24 additions & 1 deletion java/ql/lib/semmle/code/java/security/CsrfUnprotectedRequestTypeQuery.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,35 @@ private predicate sink(CallPathNode sinkMethodCall) {
)
}

Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing documentation for the fwdFlow predicate. Should explain that it computes forward reachability from sources using transitive closure over call graph edges.

Suggested change
/**
* Computes forward reachability from source nodes using transitive closure
* over call graph edges. Holds if `n` is reachable from a source node.
*/

Copilot uses AI. Check for mistakes.
private predicate fwdFlow(CallPathNode n) {
source(n)
or
exists(CallPathNode mid | fwdFlow(mid) and CallGraph::edges(mid, n))
}

Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing documentation for the revFlow predicate. Should explain that it computes backward reachability from sinks within the forward-reachable nodes, implementing the bidirectional pruning strategy.

Suggested change
/**
* Computes backward reachability from sinks within the forward-reachable nodes.
* This predicate implements the bidirectional pruning strategy by combining
* forward reachability (`fwdFlow`) with backward reachability from sinks.
*/

Copilot uses AI. Check for mistakes.
private predicate revFlow(CallPathNode n) {
fwdFlow(n) and
(
sink(n)
or
exists(CallPathNode mid | revFlow(mid) and CallGraph::edges(n, mid))
)
}

/**
* Holds if `pred` has a successor node `succ` and this edge is in an
* `unprotectedStateChange` path.
*/
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The predicate should include documentation about its performance characteristics and the bidirectional pruning strategy, as this is a key optimization that affects query behavior.

Suggested change
*/
*/
/**
* Holds if `pred` has a successor node `succ` and this edge is in an
* `unprotectedStateChange` path.
*
* Performance characteristics:
* - This predicate relies on `revFlow`, which is a recursive flow predicate
* that combines forward and reverse graph traversal.
* - The use of `CallGraph::edges` ensures that only valid edges in the call
* graph are considered, reducing unnecessary computations.
*
* Bidirectional pruning strategy:
* - The predicate leverages `revFlow(pred)` and `revFlow(succ)` to prune
* the search space by ensuring that both the predecessor and successor
* nodes are part of valid reverse flows.
*/

Copilot uses AI. Check for mistakes.
predicate relevantEdge(CallPathNode pred, CallPathNode succ) {
CallGraph::edges(pred, succ) and revFlow(pred) and revFlow(succ)
}

/**
* Holds if `sourceMethod` is an unprotected request handler that reaches a
* `sinkMethodCall` that updates a database.
*/
private predicate unprotectedDatabaseUpdate(CallPathNode sourceMethod, CallPathNode sinkMethodCall) =
doublyBoundedFastTC(CallGraph::edges/2, source/1, sink/1)(sourceMethod, sinkMethodCall)
doublyBoundedFastTC(relevantEdge/2, source/1, sink/1)(sourceMethod, sinkMethodCall)

/**
* Holds if `sourceMethod` is an unprotected request handler that appears to
Expand Down
2 changes: 1 addition & 1 deletion java/ql/src/Security/CWE/CWE-352/CsrfUnprotectedRequestType.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import java
import semmle.code.java.security.CsrfUnprotectedRequestTypeQuery

query predicate edges(CallPathNode pred, CallPathNode succ) { CallGraph::edges(pred, succ) }
query predicate edges(CallPathNode pred, CallPathNode succ) { relevantEdge(pred, succ) }

from CallPathNode source, CallPathNode sink
where unprotectedStateChange(source, sink)
Expand Down