[13.x] Use a timing-safe comparison for the maintenance mode bypass secret

[yazansalhi]

PreventRequestsDuringMaintenance compares the bypass secret to the request path with a plain string comparison:

if (isset($data['secret']) && $request->path() === $data['secret']) {
    return $this->bypassResponse($data['secret']);
}

The cookie that this branch issues is already validated with hash_equals() in MaintenanceModeBypassCookie::isValid(), so the two halves of the same bypass flow currently use different comparison styles. This makes them consistent.

The same comparison exists in stubs/maintenance-mode.stub, eleven lines above the hash_equals() call that validates the bypass cookie, so this changes both (thanks @GrahamCampbell for pointing that one out). That path runs before the framework boots.

To be clear about the scope: I am not reporting an exploitable vulnerability. No usable timing oracle exists here — === compares lengths before content, so wrong-length guesses never reach memcmp, and a short secret is a single vectorized compare with no per-byte gradient to climb. This is a defense-in-depth/consistency change in the same spirit as #21320, where the remember_me comparison was moved to hash_equals().

There is no behavior change in either file, for any input:

• Middleware: the isset() guard becomes is_string(), because the original === is strict and would never have matched a non-string secret. Without the guard, hash_equals() would raise a TypeError where the old code returned false.
• Stub: the isset() guard is kept, because there the secret is concatenated onto '/' before the comparison, so a non-string secret is already coerced to a string and continues to match exactly as before.

$data['secret']	request	before	after
'foo'	'foo'	true	true
'foo'	'bar'	false	false
not set	'foo'	false	false
123 (middleware)	'123'	false	false
123 (stub)	'/123'	true	true

The existing maintenance mode tests in tests/Integration/Foundation/MaintenanceModeTest.php continue to describe the expected behavior.

[GrahamCampbell]

I agree this isn't an exploitable vulnerability, but not for the reason you give. Entropy defends against blind guessing, not against a timing oracle: a byte-at-a-time oracle converts an exponential search into a linear one, so it sidesteps entropy entirely. Str::random() defaults to 16 characters, roughly 95 bits and unbruteforceable, but under a working prefix oracle that's about 16 × 62 ≈ 1,000 requests. What actually makes this safe is that no usable oracle exists here: === compares lengths before content, so wrong-length guesses never reach memcmp, and glibc's memcmp is vectorized at 16–32 bytes per instruction, so a 16-byte secret is a single vector compare with no per-byte gradient to climb.

Worth noting the change is incomplete, too: stubs/maintenance-mode.stub:55 does the same comparison against $_SERVER['REQUEST_URI'], eleven lines above a hash_equals() call on line 66 — the same inconsistency you're describing, in the path that runs before the framework boots.

[yazansalhi]

Thanks — you're right, the entropy reasoning was wrong; a byte-at-a-time oracle would sidestep entropy entirely. I've corrected the description.

Good catch on the stub — pushed a commit fixing that one too.