Race condition in Optimizer::serve() causes PHP warnings when concurrent requests generate the same CSS file

[unsetfocus]

Description

Optimizer::serve() in src/optimizer.cls.php contains a TOCTOU (time-of-check/time-of-use) race condition that produces PHP warnings under concurrent load when two requests attempt to generate the same combined/minified CSS file simultaneously.

Affected version

7.8.1

User-facing impact

When the race fires, md5_file() returns false, causing serve() to return ['.css', 'css'] (a filename with no hash). This has two consequences:

1. Unstyled page for the affected visitor — the browser receives <link rel="stylesheet" href=".../litespeed/css/.css?ver=xxxxx" /> which 404s, causing all combined styles to fail to load.
2. Broken URL persists in the database — Data::save_url() is called with $filecon_md5 = false and stores the broken filename. Subsequent visitors hitting the same URL may also receive the broken stylesheet reference until the LiteSpeed cache is manually purged.

Error messages observed in WP_DEBUG_LOG

PHP Warning: md5_file(/path/to/wp-content/litespeed/css/abc123.css.tmp): Failed to open stream: No such file or directory in .../litespeed-cache/src/optimizer.cls.php on line 148
PHP Warning: rename(/path/to/wp-content/litespeed/css/abc123.css.tmp, /path/to/wp-content/litespeed/css/.css): No such file or directory in .../litespeed-cache/src/optimizer.cls.php on line 153

Root cause

The guard at lines 109–114 is intended to prevent two concurrent requests from generating the same file:

$tmp_static_file = $static_file . '.tmp';
if (file_exists($tmp_static_file) && time() - filemtime($tmp_static_file) <= 600) {
    // some other request is generating
    return false;
}
File::save($tmp_static_file, '', true);

However, file_exists() and File::save() are two separate non-atomic operations. Two concurrent requests for the same URL can both pass the file_exists() check before either has created the file:

1. Request A: file_exists(tmp) → false → passes guard
2. Request B: file_exists(tmp) → false → passes guard (both slip through simultaneously)
3. Request A: File::save(tmp, '') → creates tmp file
4. Request B: File::save(tmp, '') → overwrites with empty content
5. Request A: writes CSS content, minifies, calls md5_file(tmp) → gets hash, calls rename(tmp, finalfile) → tmp file is now gone
6. Request B: md5_file(tmp) → ⚠️ false — No such file or directory (line 148)
7. Request B: rename(tmp, '.css') → ⚠️ fails — No such file or directory (line 153)

Note: File::save() uses file_put_contents(..., LOCK_EX) internally, but LOCK_EX is an advisory flock — it does not block rename(), which operates on directory entries and ignores advisory locks entirely. The lock does not protect against this race.

Steps to reproduce

The race window is narrow (between rename() in one worker and md5_file() in another), but the following PHP script confirms the bug using pcntl_fork() to synchronise two processes to the exact vulnerable moment:

<?php
// Reproducer for LiteSpeed Cache TOCTOU race in Optimizer::serve()
$tmp_dir = sys_get_temp_dir() . '/lscache-race-test';
@mkdir($tmp_dir, 0755, true);
$tmp_file    = $tmp_dir . '/abc123def456.css.tmp';
$real_prefix = $tmp_dir . '/';
$css_content = "body { color: red; } .container { max-width: 1200px; }";
$ready_a     = $tmp_dir . '/.ready_a';
$ready_b     = $tmp_dir . '/.ready_b';
foreach (glob($tmp_dir . '/*') as $f) @unlink($f);

$pid = pcntl_fork();
if ($pid === -1) die("pcntl_fork() failed\n");

$me       = ($pid === 0) ? 'Request B' : 'Request A';
$other    = ($pid === 0) ? 'Request A' : 'Request B';

// Both signal ready and wait for each other (synchronise past the guard)
file_put_contents($me === 'Request A' ? $ready_a : $ready_b, '1');
$deadline = microtime(true) + 3.0;
$other_ready = $me === 'Request A' ? $ready_b : $ready_a;
while (!file_exists($other_ready) && microtime(true) < $deadline) usleep(200);

echo "[$me] file_exists(tmp)? " . (file_exists($tmp_file) ? 'true' : 'false') . " - passes guard\n";

// Both create and write to tmp (same content - same URL = same CSS sources)
file_put_contents($tmp_file, '', LOCK_EX);
usleep(5000);
file_put_contents($tmp_file, $css_content, LOCK_EX);
echo "[$me] Wrote CSS to tmp\n";

// Request B sleeps briefly - simulates OS scheduling / longer minification
if ($me === 'Request B') usleep(20000);

echo "[$me] Calling md5_file(tmp)...\n";
$md5 = @md5_file($tmp_file);

if ($md5 === false) {
    echo "[$me] *** BUG REPRODUCED: md5_file() returned false (tmp renamed away by $other)\n";
    echo "[$me] serve() returns ['.css', 'css'] - browser gets a 404 stylesheet\n";
    @rename($tmp_file, $real_prefix . '.css');
} else {
    $realfile = $real_prefix . $md5 . '.css';
    if (!file_exists($realfile)) { rename($tmp_file, $realfile); echo "[$me] OK: $md5.css\n"; }
    else { @unlink($tmp_file); echo "[$me] OK: already exists\n"; }
}

if ($pid !== 0) {
    pcntl_wait($status);
    echo "\nFinal dir contents:\n";
    foreach (glob($tmp_dir . '/*') as $f) echo '  ' . basename($f) . "\n";
}

Expected output (bug confirmed):

[Request A] file_exists(tmp)? false - passes guard
[Request B] file_exists(tmp)? false - passes guard
[Request A] Wrote CSS to tmp
[Request B] Wrote CSS to tmp
[Request A] Calling md5_file(tmp)...
[Request A] OK: d41d8cd98f00b204e9800998ecf8427e.css
[Request B] Calling md5_file(tmp)...
[Request B] *** BUG REPRODUCED: md5_file() returned false (tmp renamed away by Request A)
[Request B] serve() returns ['.css', 'css'] - browser gets a 404 stylesheet

Suggested fix

Replace the non-atomic check/create pair with an exclusive file open using fopen(..., 'x'), which is atomic at the OS level and fails if the file already exists:

$tmp_static_file = $static_file . '.tmp';
$fh = @fopen($tmp_static_file, 'x');
if ($fh === false) {
    if (file_exists($tmp_static_file) && time() - filemtime($tmp_static_file) <= 600) {
        // some other request is generating
        return false;
    }
    // stale tmp file - proceed
} else {
    fclose($fh);
}

Alternatively, using per-request unique tmp filenames (e.g. appending getmypid() or a random suffix) would also eliminate the race entirely.

[hi-hai]

Thanks for the high quality report. The patch is in c4088bf