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

Overview

Go makes concurrency cheap, so shared state is reached from many goroutines by default rather than by exception. A check and the action it guards run as separate steps, and the scheduler can place another goroutine between them at any point - there is no equivalent of a global interpreter lock to blur the question.

Two shapes account for most findings: a read-modify-write on a shared variable where the read and write are separate operations, and a filesystem check followed by an operation on the same path.

Common Vulnerable Patterns

Check-then-act on a shared counter

// VULNERABLE - two goroutines can both observe remaining > 0
type Inventory struct {
    remaining int64
}

func (inv *Inventory) Reserve() error {
    if atomic.LoadInt64(&inv.remaining) <= 0 {
        return errors.New("out of stock")
    }

    // Another goroutine reserves the last unit here
    atomic.AddInt64(&inv.remaining, -1)
    return nil
}

Why this is vulnerable: Both operations are individually atomic, which is what makes this look correct. Atomicity of each step says nothing about the pair: the load and the add are two operations, and the value can change in between. The result is a negative count and more reservations than stock.

Existence check before file creation

// VULNERABLE - the path is resolved twice
func saveUpload(path string, data []byte) error {
    if _, err := os.Stat(path); err == nil {
        return errors.New("file already exists")
    }

    // Another process creates the path, or replaces it with a symlink, here
    return os.WriteFile(path, data, 0o600)
}

Why this is vulnerable: os.Stat reports on the path at that moment and os.WriteFile resolves it again, following symlinks. In a shared directory the write lands wherever the attacker pointed the name, and the permission argument does not apply to a file that already exists.

Secure Patterns

Compare-and-swap in a retry loop

// SECURE - the update only lands if the value is still what the check saw
func (inv *Inventory) Reserve() error {
    for {
        current := atomic.LoadInt64(&inv.remaining)
        if current <= 0 {
            return errors.New("out of stock")
        }

        if atomic.CompareAndSwapInt64(&inv.remaining, current, current-1) {
            return nil
        }
        // Another goroutine won; re-read and decide again
    }
}

Why this works: CompareAndSwapInt64 writes only if the variable still holds the value the check was based on, so a goroutine that lost the race observes the failure and re-evaluates rather than applying a decision made on a stale read. The loop is the mechanism, not a workaround. In Go 1.19 and later, atomic.Int64 with its CompareAndSwap method gives the same behaviour without raw pointer arguments.

Use a sync.Mutex instead when the critical section covers more than one variable - CAS protects a single word, and two coordinated CAS operations are not a transaction.

Bind file operations to a descriptor

//go:build unix

// SECURE - creation is atomic; O_EXCL fails rather than overwriting
func saveUpload(path string, data []byte) error {
    f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o600)
    if err != nil {
        return err  // os.ErrExist if another process got there first
    }
    defer f.Close()

    if _, err := f.Write(data); err != nil {
        return err
    }
    return f.Sync()
}

// SECURE - unpredictable name, created exclusively
func saveTemp(dir string, data []byte) (string, error) {
    f, err := os.CreateTemp(dir, "upload-*")
    if err != nil {
        return "", err
    }
    defer f.Close()

    if _, err := f.Write(data); err != nil {
        return "", err
    }
    return f.Name(), f.Sync()
}

Why this works: O_CREATE|O_EXCL performs the existence check inside the kernel, under the same lock as the creation, so a pre-existing path produces os.ErrExist instead of an overwrite. O_NOFOLLOW refuses a symlinked final component, and os.CreateTemp adds an unpredictable name to the same exclusive creation, which removes the attacker's ability to pre-place the entry. Once the file is open, further operations use the handle rather than the path, so a later rename cannot redirect them.

The //go:build unix line is load-bearing rather than tidiness. O_NOFOLLOW comes from syscall because os does not export it, and syscall.O_NOFOLLOW is undefined on Windows - not zero, undefined - so a package containing saveUpload fails to compile on a Windows builder unless the file is constrained. Put the line first in the file, above the package clause and separated from it by a blank line. saveTemp needs no such guard; os.CreateTemp is portable, so keep it in an unconstrained file if Windows builds need it.

Push the invariant into the database for shared state

// SECURE - condition and update evaluated together under the row lock
func Withdraw(ctx context.Context, db *sql.DB, accountID string, amount int64) error {
    res, err := db.ExecContext(ctx, `
        UPDATE accounts
           SET balance = balance - $1
         WHERE id = $2
           AND balance >= $1`, amount, accountID)
    if err != nil {
        return err
    }

    affected, err := res.RowsAffected()
    if err != nil {
        return err
    }
    if affected == 0 {
        return errors.New("insufficient funds")
    }
    return nil
}

Why this works: A mutex or CAS coordinates goroutines inside one process, and a Go service is normally deployed as several replicas. Expressing the condition as part of the UPDATE moves the guarantee to the one place all replicas share, and RowsAffected reports the outcome at write time rather than at read time. It does not say which part of the predicate failed: zero means no row matched id = $2 AND balance >= $1, so either the account does not exist or its balance was too low. Where the caller has to tell those apart, follow the failed update with a SELECT for the id - nothing was written, so there is nothing to undo.

Considerations

Which boundary the state crosses. In-process state takes sync.Mutex or sync/atomic; state shared between replicas takes the database. A finding "fixed" with a mutex on a multi-replica service is not fixed, and it will pass every single-instance test.
Atomic operations are not an atomic section. atomic.LoadInt64 followed by atomic.AddInt64 is the vulnerable pattern above. If the decision spans two operations, it needs CAS or a mutex - the word "atomic" in the function name applies to each call, not the sequence.
Whether the race is reachable. State confined to one goroutine, or guarded by the channel that owns it, is not this weakness. Go's convention of communicating over channels rather than sharing memory means some findings are reports on code that is already single-owner - record those as false positives with the ownership reason.
CAS loops need a bound where fairness matters. An unbounded retry under heavy contention can starve a goroutine indefinitely. A mutex is the better choice when the critical section is long or fairness matters more than throughput.

Testing

Go's race detector is the tool that makes this testable, and it is not enough on its own: it reports unsynchronised memory access, not a logically wrong interleaving between two correctly-atomic operations.

Run the concurrency tests with go test -race. Assert clean output, and treat any report as a defect rather than as noise.
Start N goroutines from a sync.WaitGroup against stock that admits exactly one reservation, and assert exactly one success and a count that never goes below zero. Release them together so they contend; a sequential loop passes against the vulnerable version.
Use t.Parallel() and -count=100 so the scheduler explores different interleavings across runs. A race test that runs once has told you very little.
Pre-create the upload path and assert errors.Is(err, os.ErrExist) and that the existing content is unchanged; point it at a symlink and assert the open fails.
Run the database test against two processes concurrently, which is the case a mutex-based fix cannot survive.