Improper Synchronization

Description

Improper Synchronization occurs when concurrent access to shared resources is not properly controlled, leading to race conditions, data corruption, or security vulnerabilities. This includes missing locks, incorrect lock scope, lock ordering issues, and time-of-check-to-time-of-use (TOCTOU) conditions. In security contexts, improper synchronization can allow attackers to manipulate program state between checks and operations.

Risk

Race conditions allow privilege escalation. TOCTOU attacks bypass access controls. Data corruption from concurrent writes. Deadlocks cause denial of service. Double-spending in financial applications. Authentication bypass through timing attacks. Session fixation through race conditions.

Solution

Use proper synchronization primitives. Implement atomic operations for check-and-act sequences. Use transactions for database operations. Apply lock ordering to prevent deadlocks. Use thread-safe data structures. Implement proper session locking. Test for race conditions with fuzzing.

Common Consequences

Impact	Details
Integrity	Scope: Data Corruption Concurrent access corrupts shared state.
Authorization	Scope: Bypass Race conditions bypass security checks.
Availability	Scope: Deadlock/DoS Improper locking causes system hangs.

Example Code + Solution Code

Vulnerable Code

// VULNERABLE: Race condition in authentication
public class VulnerableAuthService {

    private Map<String, Session> sessions = new HashMap<>();  // Not thread-safe!

    public Session login(String username, String password) {
        // VULNERABLE: TOCTOU race condition
        if (validateCredentials(username, password)) {
            // Gap between check and session creation
            Session session = new Session(username);
            sessions.put(session.getId(), session);  // Thread-unsafe write
            return session;
        }
        return null;
    }

    public boolean isLoggedIn(String sessionId) {
        // VULNERABLE: Check without synchronization
        return sessions.containsKey(sessionId);
    }
}

// VULNERABLE: Balance check race condition
public class VulnerableBankAccount {

    private double balance;  // Not atomic

    public boolean withdraw(double amount) {
        // VULNERABLE: TOCTOU - check and act not atomic
        if (balance >= amount) {
            // Another thread could withdraw between check and update
            balance -= amount;  // Non-atomic operation
            return true;
        }
        return false;
    }

    // Double withdrawal attack possible:
    // Thread A: checks balance (100), sees sufficient funds
    // Thread B: checks balance (100), sees sufficient funds
    // Thread A: withdraws 100, balance = 0
    // Thread B: withdraws 100, balance = -100 (overdraft!)
}

// VULNERABLE: File-based TOCTOU
public class VulnerableFileAccess {

    public void readFile(String filename) {
        File file = new File(filename);

        // VULNERABLE: TOCTOU race condition
        if (file.exists() && file.canRead()) {
            // Gap: attacker could replace file with symlink here
            // between check and use
            return Files.readAllBytes(file.toPath());
        }
    }
}

# VULNERABLE: Python race conditions
import threading

class VulnerableCounter:
    def __init__(self):
        self.count = 0  # Not thread-safe

    def increment(self):
        # VULNERABLE: Read-modify-write not atomic
        current = self.count
        # Context switch could happen here
        self.count = current + 1

# VULNERABLE: Authentication race condition
class VulnerableAuth:
    def __init__(self):
        self.sessions = {}  # Not thread-safe dict operations

    def login(self, user_id, password):
        # VULNERABLE: Check then act
        if self.validate(user_id, password):
            # Race: another thread could invalidate between check and create
            session = create_session(user_id)
            self.sessions[session.id] = session
            return session

    def check_session(self, session_id):
        # VULNERABLE: No synchronization
        if session_id in self.sessions:
            # Race: session could be removed between check and use
            return self.sessions[session_id]

# VULNERABLE: TOCTOU file access
import os

def process_file_vulnerable(filepath):
    # VULNERABLE: Check then use
    if os.path.exists(filepath) and os.access(filepath, os.R_OK):
        # Attacker could swap file with symlink here
        with open(filepath, 'r') as f:
            return f.read()

# VULNERABLE: Rate limiting race condition
class VulnerableRateLimiter:
    def __init__(self):
        self.counts = {}

    def is_allowed(self, user_id):
        # VULNERABLE: Not atomic
        if user_id not in self.counts:
            self.counts[user_id] = 0

        current = self.counts[user_id]
        if current < 100:
            self.counts[user_id] = current + 1  # Race condition
            return True
        return False

// VULNERABLE: Node.js race conditions
class VulnerableService {
    constructor() {
        this.data = {};
        this.sessions = new Map();
    }

    // VULNERABLE: Async race condition
    async withdraw(accountId, amount) {
        const account = await this.getAccount(accountId);

        // VULNERABLE: Check then act with async gap
        if (account.balance >= amount) {
            // Another request could process between check and update
            account.balance -= amount;
            await this.saveAccount(account);
            return true;
        }
        return false;
    }

    // VULNERABLE: Session race condition
    async createSession(userId) {
        const existingSession = await this.findSession(userId);

        // VULNERABLE: Race condition
        if (!existingSession) {
            // Two requests could both see no session and create duplicates
            const session = { id: generateId(), userId };
            this.sessions.set(session.id, session);
            return session;
        }
        return existingSession;
    }

    // VULNERABLE: Counter race
    async incrementCounter(key) {
        const value = await this.redis.get(key) || 0;
        // VULNERABLE: Not atomic
        await this.redis.set(key, parseInt(value) + 1);
    }
}

// VULNERABLE: File check then use
const fs = require('fs').promises;

async function readFileVulnerable(filepath) {
    try {
        await fs.access(filepath, fs.constants.R_OK);
        // Gap: file could change between check and read
        return await fs.readFile(filepath);
    } catch (e) {
        return null;
    }
}

// VULNERABLE: C race conditions
#include <pthread.h>

// VULNERABLE: Unprotected shared counter
int global_counter = 0;

void* increment_vulnerable(void* arg) {
    for (int i = 0; i < 1000000; i++) {
        // VULNERABLE: Read-modify-write not atomic
        global_counter++;  // Not thread-safe
    }
    return NULL;
}

// VULNERABLE: TOCTOU file access
int access_file_vulnerable(const char* filename) {
    struct stat st;

    // VULNERABLE: Check then use
    if (stat(filename, &st) == 0) {
        if (st.st_uid == getuid()) {
            // Attacker could swap file between check and open
            int fd = open(filename, O_RDONLY);
            // ...
        }
    }
}

// VULNERABLE: Double-free race condition
void* shared_ptr = NULL;
pthread_mutex_t mutex;  // But not properly used

void free_shared_vulnerable() {
    // VULNERABLE: Check then act without holding lock
    if (shared_ptr != NULL) {
        // Another thread could free between check and free
        free(shared_ptr);
        shared_ptr = NULL;
    }
}

Fixed Code

// SAFE: Proper synchronization
public class SafeAuthService {

    private final ConcurrentHashMap<String, Session> sessions = new ConcurrentHashMap<>();

    public Session login(String username, String password) {
        if (validateCredentials(username, password)) {
            Session session = new Session(username);
            // SAFE: Atomic put operation
            sessions.put(session.getId(), session);
            return session;
        }
        return null;
    }

    public boolean isLoggedIn(String sessionId) {
        // SAFE: Thread-safe check
        return sessions.containsKey(sessionId);
    }
}

// SAFE: Atomic balance operations
public class SafeBankAccount {

    private final AtomicReference<BigDecimal> balance;
    private final Object withdrawLock = new Object();

    public SafeBankAccount(BigDecimal initialBalance) {
        this.balance = new AtomicReference<>(initialBalance);
    }

    // SAFE: Using compare-and-swap
    public boolean withdrawCAS(BigDecimal amount) {
        while (true) {
            BigDecimal current = balance.get();
            if (current.compareTo(amount) < 0) {
                return false;  // Insufficient funds
            }
            BigDecimal newBalance = current.subtract(amount);
            if (balance.compareAndSet(current, newBalance)) {
                return true;  // Successfully withdrawn
            }
            // Retry if another thread modified
        }
    }

    // Or using synchronized
    public synchronized boolean withdrawSync(BigDecimal amount) {
        if (balance.get().compareTo(amount) >= 0) {
            balance.set(balance.get().subtract(amount));
            return true;
        }
        return false;
    }
}

// SAFE: Avoiding TOCTOU with atomic operations
public class SafeFileAccess {

    public byte[] readFile(Path path) {
        try {
            // SAFE: Single atomic operation, no TOCTOU
            return Files.readAllBytes(path);
        } catch (NoSuchFileException | AccessDeniedException e) {
            // Handle access issues
            return null;
        }
    }

    // SAFE: Using file locks for exclusive access
    public void writeFileExclusive(Path path, byte[] data) throws IOException {
        try (FileChannel channel = FileChannel.open(path,
                StandardOpenOption.WRITE, StandardOpenOption.CREATE)) {
            // SAFE: Exclusive lock
            try (FileLock lock = channel.lock()) {
                channel.write(ByteBuffer.wrap(data));
            }
        }
    }
}

# SAFE: Python with proper synchronization
import threading
from threading import Lock, RLock
import fcntl

class SafeCounter:
    def __init__(self):
        self.count = 0
        self.lock = Lock()

    def increment(self):
        # SAFE: Atomic operation with lock
        with self.lock:
            self.count += 1

# SAFE: Thread-safe authentication
class SafeAuth:
    def __init__(self):
        self.sessions = {}
        self.lock = RLock()

    def login(self, user_id, password):
        if self.validate(user_id, password):
            # SAFE: Synchronized session creation
            with self.lock:
                session = create_session(user_id)
                self.sessions[session.id] = session
                return session
        return None

    def check_session(self, session_id):
        # SAFE: Synchronized access
        with self.lock:
            return self.sessions.get(session_id)

# SAFE: Avoiding TOCTOU with atomic operations
def process_file_safe(filepath):
    try:
        # SAFE: Single atomic operation
        with open(filepath, 'r') as f:
            return f.read()
    except (FileNotFoundError, PermissionError):
        return None

# SAFE: File locking for exclusive access
def write_file_safe(filepath, content):
    with open(filepath, 'w') as f:
        # SAFE: Exclusive file lock
        fcntl.flock(f.fileno(), fcntl.LOCK_EX)
        try:
            f.write(content)
        finally:
            fcntl.flock(f.fileno(), fcntl.LOCK_UN)

# SAFE: Atomic rate limiting with Redis
import redis

class SafeRateLimiter:
    def __init__(self):
        self.redis = redis.Redis()

    def is_allowed(self, user_id, limit=100, window=60):
        key = f"ratelimit:{user_id}"

        # SAFE: Atomic increment with Lua script
        script = """
        local current = redis.call('INCR', KEYS[1])
        if current == 1 then
            redis.call('EXPIRE', KEYS[1], ARGV[1])
        end
        return current
        """

        current = self.redis.eval(script, 1, key, window)
        return current <= limit

// SAFE: Node.js with proper synchronization
const { Mutex } = require('async-mutex');

class SafeService {
    constructor() {
        this.mutex = new Mutex();
        this.sessions = new Map();
    }

    // SAFE: Using mutex for atomic operations
    async withdraw(accountId, amount) {
        const release = await this.mutex.acquire();
        try {
            const account = await this.getAccount(accountId);

            if (account.balance >= amount) {
                account.balance -= amount;
                await this.saveAccount(account);
                return true;
            }
            return false;
        } finally {
            release();
        }
    }

    // SAFE: Database transaction for atomicity
    async withdrawWithTransaction(accountId, amount) {
        return await this.db.transaction(async (trx) => {
            // SAFE: Row-level locking
            const account = await trx('accounts')
                .where({ id: accountId })
                .forUpdate()  // Lock row
                .first();

            if (account.balance >= amount) {
                await trx('accounts')
                    .where({ id: accountId })
                    .update({ balance: account.balance - amount });
                return true;
            }
            return false;
        });
    }

    // SAFE: Atomic session creation
    async createSession(userId) {
        const release = await this.mutex.acquire();
        try {
            // Check and create atomically
            for (const [id, session] of this.sessions) {
                if (session.userId === userId) {
                    return session;
                }
            }

            const session = { id: generateId(), userId };
            this.sessions.set(session.id, session);
            return session;
        } finally {
            release();
        }
    }

    // SAFE: Atomic Redis operations
    async incrementCounter(key) {
        // SAFE: INCR is atomic in Redis
        return await this.redis.incr(key);
    }
}

// SAFE: Atomic file operations
const fs = require('fs').promises;
const lockfile = require('proper-lockfile');

async function writeFileSafe(filepath, content) {
    // SAFE: File locking
    const release = await lockfile.lock(filepath);
    try {
        await fs.writeFile(filepath, content);
    } finally {
        await release();
    }
}

// SAFE: C with proper synchronization
#include <pthread.h>
#include <stdatomic.h>

// SAFE: Atomic counter
atomic_int safe_counter = 0;

void* increment_safe(void* arg) {
    for (int i = 0; i < 1000000; i++) {
        // SAFE: Atomic increment
        atomic_fetch_add(&safe_counter, 1);
    }
    return NULL;
}

// SAFE: Using mutex for complex operations
pthread_mutex_t balance_mutex = PTHREAD_MUTEX_INITIALIZER;
double account_balance = 0;

int withdraw_safe(double amount) {
    // SAFE: Lock before check-and-act
    pthread_mutex_lock(&balance_mutex);

    int success = 0;
    if (account_balance >= amount) {
        account_balance -= amount;
        success = 1;
    }

    pthread_mutex_unlock(&balance_mutex);
    return success;
}

// SAFE: Avoiding TOCTOU with proper file operations
int access_file_safe(const char* filename) {
    // SAFE: Open first, then check ownership
    int fd = open(filename, O_RDONLY);
    if (fd < 0) {
        return -1;
    }

    struct stat st;
    // SAFE: fstat on open file descriptor
    if (fstat(fd, &st) < 0) {
        close(fd);
        return -1;
    }

    if (st.st_uid != getuid()) {
        close(fd);
        return -1;
    }

    // Now safe to use fd
    return fd;
}

---

Exploited in the Wild

Double Spending

Race conditions in payment systems.

Privilege Escalation

TOCTOU attacks on setuid programs.

Session Hijacking

Race conditions in session management.

---

Tools to test/exploit

• Thread sanitizers (TSan).

• Race condition fuzzers.

• Static analysis for concurrency bugs.

---

CVE Examples

• CVE-2016-2782: Kernel TOCTOU race condition.

• CVE-2019-15666: Race condition in setuid.

---

References

1. MITRE. "CWE-662: Improper Synchronization." https://cwe.mitre.org/data/definitions/662.html

2. "The Art of Multiprocessor Programming."