github / github/codeql

Support Java wait/notify pattern

Offen
#6,296 5 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Java question
Vorherrschende Sprache
CodeQL
Sterne
10.1k
Forks
2.1k
Ø Merge
2 T. 15 Std.
Gemergte PRs (30 T.)
141

Beschreibung

In some cases, CodeQL may not be able to follow a data flow where tainted data is stored in a class field on a synchronized block which then calls `notify` or `notifyAll` to awake a thread which in turn end up using the tainted field into a sink.

A good example is the code injection vulnerability described in this [SonarSource Blog post](https://blog.sonarsource.com/code-vulnerabilities-in-nsa-application-revealed).

I was able to get the issue reported by adding a new TaintStep which models the wait/notify pattern by connecting fields writes on a synchronized version calling `notify` with the same field reads on a synchronized version calling `wait`:

```ql
class NotifyWaitTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
exists(MethodAccess notify, RefType t, MethodAccess wait, SynchronizedStmt notifySync, SynchronizedStmt waitSync |

notify.getMethod().getName() = ["notify", "notifyAll"] and
notify.getAnEnclosingStmt() = notifySync and
notifySync.getExpr().getType() = t and

wait.getMethod().getName() = "wait" and
wait.getAnEnclosingStmt() = waitSync and
waitSync.getExpr().getType() = t and

exists(AssignExpr write, FieldAccess read, Field f |
write.getAnEnclosingStmt() = notifySync and
write.getDest().(FieldAccess).getField() = f and
write = n1.asExpr() and

read.getAnEnclosingStmt() = waitSync and
read.getField() = f and
read = n2.asExpr()
)
)
}
}
```

The taint step may need improvements though, but I think is a good starting point.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.