Skip to content

Commit f3721eb

Browse files
Jami CogswellJami Cogswell
authored andcommitted
Java: refactor unprotectedDatabaseUpdate
1 parent 530a77e commit f3721eb

File tree

1 file changed

+16
-12
lines changed

1 file changed

+16
-12
lines changed

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,28 +215,32 @@ module CallGraph {
215215
predicate edges(CallPathNode pred, CallPathNode succ) { pred.getASuccessor() = succ }
216216
}
217217

218-
/**
219-
* Holds if `sourceMethod` is an unprotected request handler that reaches a
220-
* `sinkMethodCall` that updates a database.
221-
*/
222-
private predicate unprotectedDatabaseUpdate(CallPathNode sourceMethod, CallPathNode sinkMethodCall) {
223-
sourceMethod.asMethod() instanceof CsrfUnprotectedMethod and
218+
/** Holds if `sourceMethod` is an unprotected request handler. */
219+
private predicate source(CallPathNode sourceMethod) {
220+
sourceMethod.asMethod() instanceof CsrfUnprotectedMethod
221+
}
222+
223+
/** Holds if `sinkMethodCall` updates a database. */
224+
private predicate sink(CallPathNode sinkMethodCall) {
224225
exists(CallPathNode sinkMethod |
225226
sinkMethod.asMethod() instanceof DatabaseUpdateMethod and
226-
sinkMethodCall.getASuccessor() = pragma[only_bind_into](sinkMethod) and
227-
sourceMethod.getASuccessor+() = pragma[only_bind_into](sinkMethodCall) and
227+
sinkMethodCall.getASuccessor() = sinkMethod and
228228
// exclude SQL `execute` calls that do not update database
229229
if
230230
sinkMethod.asMethod() instanceof SqlInjectionDatabaseUpdateMethod and
231231
sinkMethod.asMethod().hasName("execute")
232-
then
233-
exists(SqlExecuteFlow::PathNode executeSink | SqlExecuteFlow::flowPath(_, executeSink) |
234-
sinkMethodCall.asCall() = executeSink.getNode().asExpr().(Argument).getCall()
235-
)
232+
then SqlExecuteFlow::flowToExpr(sinkMethodCall.asCall().getAnArgument())
236233
else any()
237234
)
238235
}
239236

237+
/**
238+
* Holds if `sourceMethod` is an unprotected request handler that reaches a
239+
* `sinkMethodCall` that updates a database.
240+
*/
241+
private predicate unprotectedDatabaseUpdate(CallPathNode sourceMethod, CallPathNode sinkMethodCall) =
242+
doublyBoundedFastTC(CallGraph::edges/2, source/1, sink/1)(sourceMethod, sinkMethodCall)
243+
240244
/**
241245
* Holds if `sourceMethod` is an unprotected request handler that appears to
242246
* change application state based on its name.

0 commit comments

Comments
 (0)