CVE-2019-11936 (PHP, published 2019-12-04T17:16Z)
Description: value: Various APC functions accept keys containing null bytes as input, leading to premature truncation of input. This issue affects HHVM versions prior to 3.30.12, all versions between 4.0.0 and 4.8.5, all versions between 4.9.0 and 4.23.1, as well as 4.24.0, 4.25.0, 4.26.0, 4.27.0, 4.28.0, and 4.28.1.
Severity: HIGH (CVSS v3 base score: 9.8)
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/facebook/hhvm
Files changed:
hphp/runtime/ext/apc/ext_apc.cpp
hphp/test/quick/apc.php
hphp/test/quick/apc.php.expectf
Commit message: Prevent APC keys with nulls

Summary:
This diff prevents storing APC keys that contain the null byte.
APC uses a backing data structure ConcurrentTableSharedStore that uses const char* as its key type. This leads to situations where inserting APC keys with null bytes will cause key truncation, like so...
localhost> p apc_clear_cache();
true
localhost> p apc_fetch("test")
false
localhost> p apc_fetch("test\x00suffix")
false
localhost> p apc_add("test\x00suffix", 5)
true
localhost> p apc_fetch("test")
5
localhost> p apc_fetch("test\x00suffix")
false
This change will make APC throw an error if a key containing a null byte is passed to the store/add methods.

Reviewed By: markw65

Differential Revision: D17608626

fbshipit-source-id: 915e179f41e66c99c718364ec4a8639d204c4ea2


Vulnerable code:
      apc_store().set(key.toString(), v, ttl);
    }
      if (!apc_store().add(key.toString(), v, ttl)) {
        errors.add(key, -1);

Fixed code:
bool isKeyInvalid(const String &key) {
  // T39154441 - check if invalid chars exist
  return key.find('\0') != -1;
}

      auto const& strKey = key.toCStrRef();
      if (isKeyInvalid(strKey)) {
        throw_invalid_argument("apc key: (contains invalid characters)");
        return Variant(false);
      }
      apc_store().set(strKey, v, ttl);
    }

  if (isKeyInvalid(strKey)) {
    throw_invalid_argument("apc key: (contains invalid characters)");
    return Variant(false);
  }
  if (isKeyInvalid(key)) {
    throw_invalid_argument("apc key: (contains invalid characters)");
    return false;
  }

      auto const& strKey = key.toCStrRef();
      if (isKeyInvalid(strKey)) {
        throw_invalid_argument("apc key: (contains invalid characters)");
        return false;
      }

      if (!apc_store().add(strKey, v, ttl)) {
        errors.add(strKey, -1);
  if (isKeyInvalid(strKey)) {
    throw_invalid_argument("apc key: (contains invalid characters)");
    return false;
  }
function testInvalidKeys() {
    // Reject keys with null bytes
    apc_add("bar\x00baz", 10);
    apc_store("test\x00xyz", "hello");
    apc_store(array("validkey" => "validvalue", "invalid\x00key" => "value"));
    foreach (array('bar', 'test', 'validkey', 'invalid') as $k) {
        var_dump(__hhvm_intrinsics\apc_fetch_no_check($k));
    }
}

  testInvalidKeys();

Warning: Invalid argument: apc key: (contains invalid characters)%S

Warning: Invalid argument: apc key: (contains invalid characters)%S

Warning: Invalid argument: apc key: (contains invalid characters)%S
bool(false)
bool(false)
string(10) "validvalue"
bool(false)