CWE-567 Unsynchronized Shared Data: Cause and Fix in Java

CWE-567 is the failure to synchronize shared data across threads. MITRE's primary example is a Java servlet that keeps request data in a static variable.

Servlets are multithreaded. The container creates one servlet instance and runs many requests through it at the same time. Any field or static variable on that class is shared by every request in flight. Developers who think of a servlet like a simple function forget this, because the framework hides the threading.

MITRE notes that this is a typical mistake in J2EE applications, and that the weakness is not unique to servlets. Any multithreaded Java code with shared mutable state can have it.

How it happens

A servlet stores something per request in a field. Two requests arrive close together. Thread A writes its value. Thread B overwrites it. Thread A then reads thread B's data.

// Vulnerable: instance field shared by all request threads
public class ProfileServlet extends HttpServlet {
    private String currentUser;          // shared state

    protected void doGet(HttpServletRequest req, HttpServletResponse res)
            throws IOException {
        currentUser = req.getParameter("user");
        // another thread may overwrite currentUser here
        res.getWriter().println("Profile for " + lookup(currentUser));
    }
}

Under light load this works every time. Under real traffic, one user sometimes sees another user's profile. That intermittent behavior is why the bug survives testing and then shows up in production as a "ghost" report nobody can reproduce.

MITRE describes the attacker angle: when a shared variable can be influenced by an attacker, one thread could modify the variable to contain data that is not valid for a different thread that is also using it.

Static variables have the same problem, with a wider blast radius, since they are shared across all instances.

What an attacker gains

MITRE lists confidentiality, integrity and availability consequences.

• Read application data. If the shared variable holds sensitive data, it may be displayed in another user's session.
• Modify application data. If the value controls application behavior, an attacker may be able to change what other users' requests do.
• Instability. Corrupted shared state can produce errors, crashes or restarts.

The attacker does not need clever tooling. Sending many concurrent requests is often enough to make the race fire.

How to find it

MITRE lists automated static analysis as a way to find some instances. Practical checks:

• Review servlet classes for fields. A servlet should have no mutable instance fields that hold per request data. Flag every non final field and every static non final field.
• Static analysis and concurrency linters. Tools that look for unguarded shared state can flag these.
• Load testing with distinct users. Run concurrent requests with different identities and verify each response only contains its own data.
• Code review of singletons. Spring beans and other container managed singletons share the same risk as servlets.

How to fix it

MITRE's mitigation: remove the use of static variables shared between servlets. If that cannot be avoided, use synchronized access for them.

In order of preference:

1. Use local variables. Anything created inside the request method is private to that thread. This is the right answer almost every time.
2. Keep per request data in the request or session object, which the container manages per request or per user.
3. Use immutable objects for shared configuration.
4. If you truly need shared mutable state, guard it with synchronized blocks, a lock, or thread safe classes such as ConcurrentHashMap and AtomicInteger, and keep the critical section small.

// Fixed: state is local to the request thread
public class ProfileServlet extends HttpServlet {
    protected void doGet(HttpServletRequest req, HttpServletResponse res)
            throws IOException {
        String user = req.getParameter("user");   // local variable
        res.getWriter().println("Profile for " + lookup(user));
    }
}

When shared state is unavoidable, such as a counter or cache:

// Shared counter done safely
private final AtomicLong hits = new AtomicLong();

protected void doGet(HttpServletRequest req, HttpServletResponse res) {
    hits.incrementAndGet();
}

Also avoid the old SingleThreadModel idea. It is deprecated and does not give you the safety people assume, since shared statics are still shared.

Concurrency bugs in libraries matter too. If you rely on third party code in a servlet container, keep track of what versions you run. The free scan at gold.safeguard.sh/scan works in your browser with no login, keeps the file on your machine, and covers up to 100 dependencies.

Quick check

Sources

• MITRE CWE-567: Unsynchronized Access to Shared Data in a Multithreaded Context
• Safeguard Gold: CWE-567