GHSA-r854-jrxh-36qx phpseclib has a variable-time HMAC comparison in SSH2::get_binary_packet() using != instead of hash_equals()

• Packagist phpseclib/phpseclib: >= 0.1.1, < 1.0.28 → fixed in 1.0.28 • Packagist phpseclib/phpseclib: >= 2.0.0, < 2.0.53 → fixed in 2.0.53 • Packagist phpseclib/phpseclib: >= 3.0.0, < 3.0.51 → fixed in 3.0.51

phpseclib SSH2: Variable-time comparison in HMAC verification Summary `phpseclib\Net\SSH2::get_binary_packet()` uses PHP's `!=` operator to compare a received SSH packet HMAC against the locally computed HMAC. `!=` on equal-length binary strings in PHP uses `memcmp()`, which short-circuits on the first differing byte. This is a real variable-time comparison (CWE-208), proven by scaling benchmarks. The finding is Low severity (defense-in-depth), not Critical. Practical exploitation over the network is prevented by SSH's disconnect-on-MAC-failure behavior combined with per-connection session keys. The fix is a one-liner: replace `!=` with `hash_equals()`, which the codebase already uses in 9 other places. • Target: `phpseclib/phpseclib` • File: `phpseclib/Net/SSH2.php` • Lines (master `e819a163c`): 3405 and 3410 • CWE: CWE-208 (Observable Timing Discrepancy) • CVSS v3.1: 3.7 — `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:N` • Severity: Low (cryptographic hygiene / defense-in-depth) • Affected branches: `master`, `3.0`, `2.0`, `1.0` (all supported versions) Root cause `phpseclib/Net/SSH2.php` lines 3399-3415 (master at `e819a163c`): ```php if ($this->hmac_check instanceof Hash) { $reconstructed = !$this->hmac_check_etm ? pack('Na*', $packet->packet_length, $packet->plain) : substr($packet->raw, 0, -$this->hmac_size); if (($this->hmac_check->getHash() & "\xFF\xFF\xFF\xFF") == 'umac') { $this->hmac_check->setNonce("\0\0\0\0" . pack('N', $this->get_seq_no)); if ($hmac != $this->hmac_check->hash($reconstructed)) { // disconnect_helper(DisconnectReason::MAC_ERROR); throw new ConnectionClosedException('Invalid UMAC'); } } else { if ($hmac != $this->hmac_check->hash(pack('Na*', $this->get_seq_no, $reconstructed))) { // disconnect_helper(DisconnectReason::MAC_ERROR); throw new ConnectionClosedException('Invalid HMAC'); } } } ``` Both `$hmac` (read from the socket via `Strings::pop($raw, $this->hmac_size)` at line 3348) and the computed hash are equal-length binary strings. PHP's `!=` operator on equal-length strings dispatches to `zend_binary_strcmp()` which internally calls `memcmp()`. Modern libc `memcmp` short-circuits on the first differing byte, producing a timing signal that scales linearly with the number of matching leading bytes. The same bug exists on every supported branch | Branch | File | Line(s) | Expression | |---|---|---|---| | `master` (@ `e819a163c`) | `phpseclib/Net/SSH2.php` | 3405, 3410 | `$hmac != $this->hmac_check->hash(...)` | | `3.0` | `phpseclib/Net/SSH2.php` | 3741, 3746 | `$hmac != $this->hmac_check->hash(...)` | | `2.0` | `phpseclib/Net/SSH2.php` | 3796 | `$hmac != $this->hmac_check->hash(...)` | | `1.0` | `phpseclib/Net/SSH2.php` | 3810 | `$hmac != $this->hmac_check->hash(...)` | Verified with `git show :phpseclib/Net/SSH2.php`. Reachability The HMAC verification path at lines 3399-3415 is reached on every received SSH packet when the negotiated cipher is not AEAD — i.e., any of: • `aes128-cbc`, `aes192-cbc`, `aes256-cbc` • `aes128-ctr`, `aes192-ctr`, `aes256-ctr` • `3des-cbc`, `3des-ctr` • `blowfish-cbc`, `blowfish-ctr` • `twofish-*-cbc`, `twofish-*-ctr` • `arcfour`, `arcfour128`, `arcfour256` combined with any non-AEAD MAC (hmac-sha2-256, hmac-sha2-512, hmac-sha1, hmac-sha1-96, hmac-md5, hmac-md5-96, umac-64, umac-128, and their -etm variants — full list at `getSupportedMACAlgorithms()` around line 4754). AEAD ciphers (`[email protected]`, `[email protected]`, `[email protected]`) go through a different path (lines 3353-3381) and do not reach the `!=` comparison — their authentication tag is checked inside the AEAD implementation. `$this->hmac_check` is set during key exchange at `SSH2.php:1812`: ```php if (!$this->decrypt->usesNonce()) { [$this->hmac_check, $checkKeyLength] = self::mac_algorithm_to_hash_instance($mac_algorithm_in); $this->hmac_size = $this->hmac_check->getLengthInBytes(); } ``` So any SSH2 client session negotiating a non-AEAD cipher exercises the vulnerable code path starting from the first post-KEX packet. Contrast with existing code that does the right thing `hash_equals()` is already used in 9 other places in the phpseclib codebase, proving the maintainer knows the pattern: ``` phpseclib/File/CMS/DigestedData.php:186 phpseclib/File/CMS/SignedData/Signer.php:294 phpseclib/File/CMS/SignedData/Signer.php:406 phpseclib/Crypt/Common/Formats/Keys/PuTTY.php:236 phpseclib/Crypt/RSA/PublicKey.php:98, 105, 188, 229 phpseclib/Crypt/RSA/PrivateKey.php:386 ``` The SSH2 MAC comparison is the one place it was missed. Proof of concept All scripts under `poc/`. They prove three things: 1. PHP's `!=` on equal-length binary strings IS variable-time and short-circuits. 2. `hash_equals()` is constant-time over the same inputs. 3. Applied to 32-byte HMAC-SHA256 values as produced by phpseclib's `Crypt\Hash` class, the code path at `SSH2.php:3405/3410` leaks ~2-14 ns of signal per comparison. PoC 1: baseline measurement (`poc/01_verify_variable_time.php`) Measures `!=` vs `hash_equals()` on 32-byte strings with first-byte vs last-byte mismatch. 500,000 iterations each, trimmed mean, Welch's t-test. Representative output (PHP 8.3.6 on Linux x86_64): ``` === PHP != operator vs hash_equals() on 32-byte strings === Iterations: 500000 != first-byte mismatch: median=40 p25=40 p75=50 tmean=44.3 != last-byte mismatch: median=50 p25=40 p75=50 tmean=46.4 Gap (last - first): tmean_delta=2.1 ns hash_equals first-byte: median=110 tmean=112.7 hash_equals last-byte: median=111 tmean=114.9 Gap (last - first): tmean_delta=2.2 ns ``` The 32-byte delta is small (~2 ns) and lives in the noise. This alone doesn't prove variable-time behavior. PoC 2 does. PoC 2: scaling test (`poc/02_scaling_test.php`) The decisive test. If `!=` is truly constant-time, timing should not depend on mismatch position. If it short-circuits, timing should scale linearly with prefix length. Test strings from 32 bytes to 4096 bytes. ``` Length 32 bytes: range (first-last byte mismatch) = 1.72 ns Length 64 bytes: range = 2.42 ns Length 128 bytes: range = 8.33 ns Length 256 bytes: range = 16.91 ns Length 512 bytes: range = 38.89 ns Length 1024 bytes: range = 81.16 ns Length 4096 bytes: range = 284.26 ns ``` This is monotone, linear scaling. Confirmed variable-time: `!=` short-circuits. Per-byte delta ≈ 0.089 ns/byte (4096 bytes → ~284 ns → 284/4096 ≈ 0.069 ns/byte after accounting for the fixed call overhead). PoC 3: contrast with `hash_equals()` (`poc/03_hash_equals_scaling.php`) Same test, `!=` vs `hash_equals()` on 1024-byte strings: ``` Mismatch pos | != tmean (ns) | hash_equals tmean ---------------------------------------------------------------------- byte 0 | 83.72 (sd 4.79) | 873.78 (sd 100.71) byte 128 | 98.48 (sd 8.27) | 855.10 (sd 87.50) byte 256 | 103.36 (sd 7.01) | 869.21 (sd 89.61) byte 512 | 125.65 (sd 10.07) | 879.74 (sd 81.82) byte 768 | 145.73 (sd 9.96) | 848.59 (sd 84.34) byte 1023 | 175.33 (sd 14.31) | 852.24 (sd 85.81) != range (last - first): 91.62 ns hash_equals range (last - first): 31.15 ns ``` `!=` monotonically increases with mismatch position. `hash_equals` is flat; the 31 ns range is measurement jitter (sd ≈ 90 ns > range). Per-byte delta from this run: 91.62 / 1023 ≈ 0.089 ns/byte. Extrapolated to 32-byte HMAC: ~2.86 ns total signal between first-byte-diff and last-byte-diff. PoC 4: end-to-end with phpseclib's Hash class (`poc/04_phpseclib_in_con…