java: add Escaping query (P1)

This commit is contained in:
yoff
2025-05-15 13:16:51 +02:00
parent 328b53576a
commit 5b30153113
7 changed files with 88 additions and 0 deletions

View File

@@ -0,0 +1,34 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
In a thread-safe class, non-final fields should generally be private (or possibly volatile) to ensure that they cannot be accessed by other threads in an unsafe manner.
</p>
</overview>
<recommendation>
<p>
If the field does not change, mark it as <code>final</code>. If the field is mutable, mark it as <code>private</code> and provide properly synchronized accessors.</p>
</recommendation>
<references>
<li>
Java Language Specification, chapter 17:
<a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
</li>
<li>
Java concurrency package:
<a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,26 @@
/**
* @name Escaping
* @description In a thread-safe class, care should be taken to avoid exposing mutable state.
* @kind problem
* @problem.severity warning
* @precision high
* @id java/escaping
* @tags quality
* reliability
* concurrency
*/
import java
import semmle.code.java.ConflictingAccess
from Field f, ClassAnnotatedAsThreadSafe c
where
f = c.getAField() and
not f.isFinal() and // final fields do not change
not f.isPrivate() and
// We believe that protected fields are also dangerous
// Volatile fields cannot cause data races, but it is dubious to allow changes.
// For now, we ignore volatile fields, but there are likely bugs to be caught here.
not f.isVolatile()
select f, "The class $@ is marked as thread-safe, but this field is potentially escaping.", c,
c.getName()

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `java/escaping`, to detect values escaping from classes marked as `@ThreadSafe`.

View File

@@ -0,0 +1,3 @@
| Escaping.java:3:7:3:7 | x | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
| Escaping.java:4:14:4:14 | y | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
| Escaping.java:9:18:9:18 | b | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |

View File

@@ -0,0 +1,17 @@
@ThreadSafe
public class Escaping {
int x; //$ Alert
public int y = 0; //$ Alert
private int z = 3;
final int w = 0;
public final int u = 4;
private final long a = 5;
protected long b = 0; //$ Alert
protected final long c = 0L;
volatile long d = 3;
protected volatile long e = 3L;
public void methodLocal() {
int i;
}
}

View File

@@ -0,0 +1,2 @@
query: Likely Bugs/Concurrency/Escaping.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql

View File

@@ -0,0 +1,2 @@
public @interface ThreadSafe {
}