phpseclib has a variable-time HMAC comparison in SSH2::get_binary_packet() using != instead of hash_equals()
# 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)) { // <-- line 3405
$this->disconnect_helper(DisconnectReason::MAC_ERROR);
throw new ConnectionClosedException('Invalid UMAC');
}
} else {
if ($hmac != $this->hmac_check->hash(pack('Na*', $this->get_seq_no, $reconstructed))) { // <-- line 3410
$this->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 <branch>: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-cbcaes128-ctr, aes192-ctr, aes256-ctr3des-cbc, 3des-ctrblowfish-cbc, blowfish-ctrtwofish-*-cbc, twofish-*-ctrarcfour, 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 thr
