Skip to content

CWE-367: Time-of-check Time-of-use Race Condition - Java

Overview

In a Java web application the two ends of this weakness are usually an authorization check and the operation it guards, separated by enough work that another request can change the state the check relied on. Every request runs on its own thread against shared state - a session object, a cached user, a database row - so "checked a moment ago" is not the same claim as "true now".

The same pattern appears in file handling, where Files.exists() followed by Files.newOutputStream() resolves the path twice.

Common Vulnerable Patterns

Authorization checked against a cached user

// VULNERABLE - the check reads session state; the operation runs later
@RestController
public class AdminController {

    @PostMapping("/admin/delete-user")
    public ResponseEntity<?> deleteUser(@RequestParam String userId, HttpSession session) {
        User currentUser = (User) session.getAttribute("user");

        if (!currentUser.isAdmin()) {
            return ResponseEntity.status(403).body("Access denied");
        }

        // Concurrent request revokes the role here; this thread never re-reads it
        userService.deleteUser(userId);

        return ResponseEntity.ok("User deleted");
    }
}

Why this is vulnerable: currentUser is a snapshot taken when the session attribute was written, so isAdmin() reports what was true at login, not what is true now. An attacker with a soon-to-be-revoked role can hold a request open and land the privileged call after revocation. The same applies to any permission cache: the check is only as fresh as the object it reads.

Existence check before file use

// VULNERABLE - two path resolutions with a window between them
public void writeReport(Path path, byte[] content) throws IOException {
    if (Files.exists(path)) {
        throw new FileAlreadyExistsException(path.toString());
    }

    // Another process creates or symlinks the path here
    Files.write(path, content);
}

Why this is vulnerable: Files.exists() and Files.write() each resolve the path independently, and Files.write() follows symbolic links by default. In a shared or world-writable directory this becomes an overwrite of whatever the attacker points the name at.

Secure Patterns

Re-check authorization inside the transaction that acts

import org.springframework.transaction.annotation.Transactional;

@Service
public class UserAdminService {

    // SECURE - the authorization read and the delete are one atomic unit
    @Transactional
    public void deleteUser(String actingUserId, String targetUserId) {
        User actor = userRepository.findByIdForUpdate(actingUserId)
                .orElseThrow(() -> new AccessDeniedException("Unknown actor"));

        if (!actor.isEnabled() || !actor.hasRole(Role.ADMIN)) {
            throw new AccessDeniedException("Not permitted");
        }

        userRepository.deleteById(targetUserId);
    }
}

Why this works: The role is read from the database inside the same transaction that performs the delete, and findByIdForUpdate (a SELECT ... FOR UPDATE via @Lock(LockModeType.PESSIMISTIC_WRITE)) holds the row until commit. A concurrent revocation either lands before the read - in which case the check fails - or waits for the lock and applies afterwards. There is no ordering in which the delete proceeds on a revoked role. Reading the role from the session or a cache would reintroduce the gap, because neither participates in the transaction.

Optimistic locking for check-then-update on a value

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Version;

@Entity
public class BankAccount {
    @Id
    private Long id;

    private BigDecimal balance;

    @Version  // JPA increments and verifies this on every update
    private Long version;

    // getters and setters
}

@Service
public class BankService {

    // SECURE - a concurrent modification makes the commit fail rather than overwrite
    @Transactional
    public void withdraw(Long accountId, BigDecimal amount) {
        BankAccount account = accountRepository.findById(accountId)
                .orElseThrow(AccountNotFoundException::new);

        if (account.getBalance().compareTo(amount) < 0) {
            throw new InsufficientFundsException();
        }

        account.setBalance(account.getBalance().subtract(amount));
        accountRepository.saveAndFlush(account);
    }
}

Why this works: @Version turns the write into UPDATE ... SET balance = ?, version = version + 1 WHERE id = ? AND version = ?. If another transaction committed between the read and the write, no row matches the version predicate, and the write fails instead of storing a value derived from stale data. The check and the update are not literally simultaneous, but the outcome is: an interleaving cannot be committed.

Name the right exception, and catch it in the right place. With a Spring Data repository the conflict does not arrive as jakarta.persistence.OptimisticLockException - the repository proxy translates it, and what reaches the caller is org.springframework.orm.ObjectOptimisticLockingFailureException, a subclass of org.springframework.dao.OptimisticLockingFailureException. A catch clause or a @Retryable(retryFor = ...) naming the JPA type never fires, so the conflict propagates straight through the handler written to absorb it; the JPA type is what a bare EntityManager.flush() throws. The catch also has to sit outside this bean, because swallowing the failure inside the @Transactional method leaves the transaction marked rollback-only and the caller gets UnexpectedRollbackException at commit instead. Both were measured on Spring Boot 3.5.5 - see CWE-362 for Java for the retry wiring. Catching and ignoring the conflict restores the original defect.

saveAndFlush rather than save is deliberate: save only queues the write, so the version check runs at commit, after the method body has returned and after any local handling could see it.

Pessimistic locking (SELECT FOR UPDATE) makes concurrent writers wait; the optimistic version lets them proceed and fails the loser. Choose optimistic when conflicts are rare and retries are cheap, pessimistic when a retry storm would be worse than waiting.

Bind file operations to the file, not the path

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Set;

// SECURE - creation is atomic and symlinks are refused
public void writeReport(Path path, byte[] content) throws IOException {
    try (var channel = Files.newByteChannel(path,
            Set.of(StandardOpenOption.CREATE_NEW,   // fails if it already exists
                   StandardOpenOption.WRITE,
                   LinkOption.NOFOLLOW_LINKS))) {
        channel.write(ByteBuffer.wrap(content));
    }
}

Why this works: CREATE_NEW performs the existence check and the creation in one filesystem operation, so a concurrently created file causes FileAlreadyExistsException rather than an overwrite. NOFOLLOW_LINKS refuses a path whose final component is a symlink. For directory-relative work under an attacker-influenced tree, SecureDirectoryStream (available from the default provider on Linux) gives openat-style operations relative to an open directory, which is the closest Java equivalent to holding a descriptor.

Considerations

  • Whether the state can actually change during the window. A check against an immutable value, or against state only this request can modify, is not this weakness. Roles, balances, quotas, session validity and file paths in shared directories are the ones that move. Say which one, and record a false positive with the reason when nothing can.
  • Locking placement, not lock existence. A synchronized block around the check does nothing if the service runs on more than one instance, and nothing if the state lives in the database. Correctness here comes from where the boundary is - the same transaction, the same row lock - not from the presence of a lock keyword.
  • Retry policy is part of the fix for optimistic locking. A bounded retry with backoff is usually right; an unbounded retry converts a correctness problem into an availability one under contention.
  • @Transactional on a private or self-invoked method does nothing. Spring proxies the bean, so an internal call bypasses the advice, and the "atomic" block silently is not one. Verify the transaction is actually applied before concluding the finding is fixed.

Testing

A re-scan can confirm the check moved; it cannot confirm the interleaving is gone. These assertions need concurrency.

  • Fire the privileged operation and the revocation concurrently, several hundred times, and assert the operation never succeeds after the revocation commits. Use a CountDownLatch so both threads start together - staggered starts miss the window.
  • Run two concurrent withdrawals against a balance that only covers one. Assert exactly one succeeds, one raises ObjectOptimisticLockingFailureException (or blocks and then fails the balance check), and the final balance is never negative. Assert on that type rather than on Exception: a test that accepts any failure passes against a handler catching the wrong exception class.
  • Assert the retry path: with a bounded retry in place, a conflicting update should eventually succeed rather than surfacing the exception to the caller.
  • Point the report path at a symlink and assert IOException rather than a write to the target.
  • Verify the transaction boundary is real - assert that the authorization read hits the database rather than a cache, for example by asserting the query count or by mutating the row from another connection mid-test.

Additional Resources