VNX-366: Race Condition within a Thread

Field	Value
Rule ID	VNX-366
Severity	Low
SARIF level	note
Kind	sast
Languages	Java
CWE	CWE-366
CAPEC	CAPEC-66
ATT&CK	T1190
CWSS vector	CWSS:1.0/TI:L/AP:RU/AL:S/IC:N/FC:LT/RP:RU/RL:S/AV:L/AS:N/IN:A/SC:A/BI:L/DI:H/EX:L/EC:N/P:L
Tags	race-in-thread concurrency cwe-366
Source	internal/sast/rules/vnx-366.rego

Overview

VNX-366 maps to CWE-366: Race Condition within a Thread. Mutable state shared by every thread is written without synchronisation, in Java. Two shapes are reported. First, a request handler of a shared singleton writes an instance field: a servlet (a class extending HttpServlet or GenericServlet) assigning, incrementing or compound-assigning a non-static, non-final field (also through this.) inside doGet/doPost/doPut/doDelete/doPatch/doHead/service, or a Spring @Controller/@RestController doing the same inside a method annotated @GetMapping/@PostMapping/@PutMapping/@DeleteMapping/@PatchMapping/@RequestMapping. One instance serves all concurrent requests, so one user’s value leaks into another’s response. Second, in a file that runs on several threads (implements Runnable, extends Thread, new Thread, an ExecutorService, submit, CompletableFuture, parallelStream, ForkJoinPool, or a servlet or Spring controller), a static, non-final, non-volatile numeric field is incremented, decremented or compound-assigned (++, –, +=) outside any synchronized method or block and without a Lock, so concurrent updates are lost. Not reported: locals, final, volatile and Atomic* fields, writes in init()/constructors, plain assignments to static fields, updates inside synchronized code, files that start no threads, and test files.

Why This Matters

A servlet or Spring controller is a singleton: one instance serves every concurrent request, so a field written by a handler leaks one user’s value into another user’s response. A static counter updated with ++ or += from several threads loses updates.

What Gets Flagged

public class LoginServlet extends HttpServlet {
    private String currentUser;

    protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
        currentUser = req.getParameter("user");   // FLAGGED
    }
}

public class Downloader implements Runnable {
    private static int completed = 0;
    public void run() {
        completed++;                               // FLAGGED
    }
}

What Is Not Flagged

private static final AtomicInteger completed = new AtomicInteger();
public void run() { completed.incrementAndGet(); }

protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
    String user = req.getParameter("user");      // a local, not a field
}

Locals (including for, catch and try-with-resources variables, and method locals of a two-space-indented file, which sit where a four-space file’s fields would), handler parameters that shadow a field (unless written through this.), annotation arguments such as @RequestParam(value = "q"), final, volatile and Atomic* fields, writes in init() and constructors, updates inside synchronized code, files that start no threads and test files are not reported.

Remediation

1. Keep per-request state in locals or request attributes, never in a servlet or controller field.
2. Use AtomicInteger/LongAdder, or guard the update with synchronized or a Lock.

References

• CWE-366: Race Condition within a Thread

Suppressing this rule

Put a nosec comment on the line, or on line 1 to cover the whole file. Use the comment marker of the file’s language (#, //, -- or ;):

# nosec vnx-366
// nosec vnx-366

Or record an ignore rule: vulnetix ignore add --rule vnx-366 --reason "...". See Suppressing findings.