Skip to content
Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
leikahing authored and facebook-github-bot committed Oct 28, 2019
1 parent 6a6eb35 commit f57df6d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
38 changes: 34 additions & 4 deletions hphp/runtime/ext/apc/ext_apc.cpp
Expand Up @@ -95,6 +95,11 @@ ConcurrentTableSharedStore& apc_store() {
return *static_cast<ConcurrentTableSharedStore*>(vpStore);
}

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

}

//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -307,9 +312,14 @@ Variant HHVM_FUNCTION(apc_store,
return Variant(false);
}
Variant v = iter.second();
apc_store().set(key.toString(), v, ttl);
}

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);
}
return Variant(ArrayData::Create());
}

Expand All @@ -318,6 +328,11 @@ Variant HHVM_FUNCTION(apc_store,
return Variant(false);
}
String strKey = key_or_array.toString();

if (isKeyInvalid(strKey)) {
throw_invalid_argument("apc key: (contains invalid characters)");
return Variant(false);
}
apc_store().set(strKey, var, ttl);
return Variant(true);
}
Expand All @@ -330,6 +345,10 @@ bool HHVM_FUNCTION(apc_store_as_primed_do_not_use,
const String& key,
const Variant& var) {
if (!apcExtension::Enable) return false;
if (isKeyInvalid(key)) {
throw_invalid_argument("apc key: (contains invalid characters)");
return false;
}
apc_store().setWithoutTTL(key, var);
return true;
}
Expand All @@ -353,8 +372,15 @@ Variant HHVM_FUNCTION(apc_add,
return false;
}
Variant v = iter.second();
if (!apc_store().add(key.toString(), v, ttl)) {
errors.add(key, -1);

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);
}
}
return errors.toVariant();
Expand All @@ -365,6 +391,10 @@ Variant HHVM_FUNCTION(apc_add,
return false;
}
String strKey = key_or_array.toString();
if (isKeyInvalid(strKey)) {
throw_invalid_argument("apc key: (contains invalid characters)");
return false;
}
return apc_store().add(strKey, var, ttl);
}

Expand Down
11 changes: 11 additions & 0 deletions hphp/test/quick/apc.php
Expand Up @@ -61,6 +61,16 @@ function testKeyTypes() {
}
}

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));
}
}

<<__EntryPoint>> function main(): void {
testApc(array(7, 4, 1776));
testApc(array("sv0", "sv1"));
Expand All @@ -73,4 +83,5 @@ function testKeyTypes() {
var_dump($b);

testKeyTypes();
testInvalidKeys();
}
10 changes: 10 additions & 0 deletions hphp/test/quick/apc.php.expectf
Expand Up @@ -72,3 +72,13 @@ string(3) "two"
Undefined index: 3
Undefined index: 2
string(5) "three"

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)

0 comments on commit f57df6d

Please sign in to comment.