Skip to content
Permalink
Browse files Browse the repository at this point in the history
Don't allow for unlimited nesting in FBUnserialize or fb_compact_unserialize
  • Loading branch information
jjergus committed Jun 30, 2020
1 parent b862493 commit 1746dfb
Show file tree
Hide file tree
Showing 9 changed files with 286 additions and 31 deletions.
1 change: 1 addition & 0 deletions hphp/hack/hhi/builtins_fb.hhi
Expand Up @@ -5,6 +5,7 @@ const FB_UNSERIALIZE_NONSTRING_VALUE = 0;
const FB_UNSERIALIZE_UNEXPECTED_END = 0;
const FB_UNSERIALIZE_UNRECOGNIZED_OBJECT_TYPE = 0;
const FB_UNSERIALIZE_UNEXPECTED_ARRAY_KEY_TYPE = 0;
const FB_UNSERIALIZE_MAX_DEPTH_EXCEEDED = 0;

const FB_SERIALIZE_HACK_ARRAYS = 0;
const FB_SERIALIZE_HACK_ARRAYS_AND_KEYSETS = 0;
Expand Down
36 changes: 21 additions & 15 deletions hphp/runtime/ext/fb/FBSerialize/FBSerialize-inl.h
Expand Up @@ -406,7 +406,7 @@ inline typename V::VariantType FBUnserializer<V>::unserialize(
folly::StringPiece serialized) {

FBUnserializer<V> unserializer(serialized);
return unserializer.unserializeThing();
return unserializer.unserializeThing(0);
}

template <class V>
Expand Down Expand Up @@ -524,7 +524,7 @@ inline FBSerializeBase::Code FBUnserializer<V>::nextCode() const {
}

template <class V>
inline typename V::MapType FBUnserializer<V>::unserializeMap() {
inline typename V::MapType FBUnserializer<V>::unserializeMap(size_t depth) {
p_ += CODE_SIZE;

typename V::MapType ret = V::createMap();
Expand All @@ -536,14 +536,14 @@ inline typename V::MapType FBUnserializer<V>::unserializeMap() {
case FB_SERIALIZE_STRING:
{
auto key = unserializeString();
auto value = unserializeThing();
auto value = unserializeThing(depth + 1);
V::mapSet(ret, std::move(key), std::move(value));
}
break;
default:
{
auto key = unserializeInt64();
auto value = unserializeThing();
auto value = unserializeThing(depth + 1);
V::mapSet(ret, std::move(key), std::move(value));
}
}
Expand All @@ -556,22 +556,23 @@ inline typename V::MapType FBUnserializer<V>::unserializeMap() {
}

template <class V>
inline typename V::VectorType FBUnserializer<V>::unserializeVector() {
inline typename V::VectorType
FBUnserializer<V>::unserializeVector(size_t depth) {
p_ += CODE_SIZE;

typename V::VectorType ret = V::createVector();

size_t code = nextCode();
while (code != FB_SERIALIZE_STOP) {
V::vectorAppend(ret, unserializeThing());
V::vectorAppend(ret, unserializeThing(depth + 1));
code = nextCode();
}
p_ += CODE_SIZE;
return ret;
}

template <class V>
inline typename V::VectorType FBUnserializer<V>::unserializeList() {
inline typename V::VectorType FBUnserializer<V>::unserializeList(size_t depth) {
p_ += CODE_SIZE;

// the list size is written so we can reserve it in the vector
Expand All @@ -582,15 +583,15 @@ inline typename V::VectorType FBUnserializer<V>::unserializeList() {

size_t code = nextCode();
while (code != FB_SERIALIZE_STOP) {
V::vectorAppend(ret, unserializeThing());
V::vectorAppend(ret, unserializeThing(depth + 1));
code = nextCode();
}
p_ += CODE_SIZE;
return ret;
}

template <class V>
inline typename V::SetType FBUnserializer<V>::unserializeSet() {
inline typename V::SetType FBUnserializer<V>::unserializeSet(size_t depth) {
p_ += CODE_SIZE;

// the set size is written so we can reserve it in the set
Expand All @@ -601,7 +602,7 @@ inline typename V::SetType FBUnserializer<V>::unserializeSet() {

size_t code = nextCode();
while (code != FB_SERIALIZE_STOP) {
V::setAppend(ret, unserializeThing());
V::setAppend(ret, unserializeThing(depth + 1));
code = nextCode();
}
p_ += CODE_SIZE;
Expand Down Expand Up @@ -664,7 +665,12 @@ inline folly::StringPiece FBUnserializer<V>::getSerializedMap() {
}

template <class V>
inline typename V::VariantType FBUnserializer<V>::unserializeThing() {
inline typename V::VariantType
FBUnserializer<V>::unserializeThing(size_t depth) {
if (UNLIKELY(depth > 1024)) {
throw UnserializeError("depth > 1024");
}

size_t code = nextCode();

switch (code) {
Expand All @@ -677,7 +683,7 @@ inline typename V::VariantType FBUnserializer<V>::unserializeThing() {
case FB_SERIALIZE_STRING:
return V::fromString(unserializeString());
case FB_SERIALIZE_STRUCT:
return V::fromMap(unserializeMap());
return V::fromMap(unserializeMap(depth));
case FB_SERIALIZE_NULL:
++p_;
return V::createNull();
Expand All @@ -686,11 +692,11 @@ inline typename V::VariantType FBUnserializer<V>::unserializeThing() {
case FB_SERIALIZE_BOOLEAN:
return V::fromBool(unserializeBoolean());
case FB_SERIALIZE_VECTOR:
return V::fromVector(unserializeVector());
return V::fromVector(unserializeVector(depth));
case FB_SERIALIZE_LIST:
return V::fromVector(unserializeList());
return V::fromVector(unserializeList(depth));
case FB_SERIALIZE_SET:
return V::fromSet(unserializeSet());
return V::fromSet(unserializeSet(depth));
default:
throw UnserializeError("Invalid code: " + folly::to<std::string>(code)
+ " at location " + folly::to<std::string>(p_));
Expand Down
10 changes: 5 additions & 5 deletions hphp/runtime/ext/fb/FBSerialize/FBSerialize.h
Expand Up @@ -165,14 +165,14 @@ struct FBUnserializer : private FBSerializeBase {
double unserializeDouble();
typename V::StringType unserializeString();
folly::StringPiece unserializeStringPiece();
typename V::MapType unserializeMap();
typename V::VectorType unserializeVector();
typename V::VectorType unserializeList();
typename V::SetType unserializeSet();
typename V::MapType unserializeMap(size_t depth);
typename V::VectorType unserializeVector(size_t depth);
typename V::VectorType unserializeList(size_t depth);
typename V::SetType unserializeSet(size_t depth);
// read the next map but don't unserialze it (for lazy or delay
// unserialization)
folly::StringPiece getSerializedMap();
typename V::VariantType unserializeThing();
typename V::VariantType unserializeThing(size_t depth);

void advance(size_t delta);
Code nextCode() const;
Expand Down
8 changes: 3 additions & 5 deletions hphp/runtime/ext/fb/VariantController.h
Expand Up @@ -283,12 +283,10 @@ struct VariantControllerImpl {
return set.size();
}
static void setAppend(SetType& set, const VariantType& v) {
auto value_type = type(v);
if (value_type != HPHP::serialize::Type::INT64 &&
value_type != HPHP::serialize::Type::STRING) {
if (!v.isInteger() && !v.isString()) {
throw HPHP::serialize::UnserializeError(
"Unsupported keyset element of type " +
folly::to<std::string>(value_type));
"Keysets can only contain integers or strings"
);
}
set.append(v);
}
Expand Down
19 changes: 13 additions & 6 deletions hphp/runtime/ext/fb/ext_fb.cpp
Expand Up @@ -66,6 +66,7 @@ static const UChar32 SUBSTITUTION_CHARACTER = 0xFFFD;
#define FB_UNSERIALIZE_UNEXPECTED_END 0x0002
#define FB_UNSERIALIZE_UNRECOGNIZED_OBJECT_TYPE 0x0003
#define FB_UNSERIALIZE_UNEXPECTED_ARRAY_KEY_TYPE 0x0004
#define FB_UNSERIALIZE_MAX_DEPTH_EXCEEDED 0x0005

#ifdef FACEBOOK
# define HHVM_FACEBOOK true
Expand Down Expand Up @@ -705,7 +706,10 @@ int fb_compact_unserialize_int64_from_buffer(
const StaticString s_empty("");

int fb_compact_unserialize_from_buffer(
Variant& out, const char* buf, int n, int& p) {
Variant& out, const char* buf, int n, int& p, size_t depth) {
if (UNLIKELY(depth > 1024)) {
return FB_UNSERIALIZE_MAX_DEPTH_EXCEEDED;
}

CHECK_ENOUGH(1, p, n);
int code = (unsigned char)buf[p];
Expand Down Expand Up @@ -774,7 +778,8 @@ int fb_compact_unserialize_from_buffer(
Array arr = Array::CreateVArray();
while (p < n && buf[p] != (char)(kCodePrefix | FB_CS_STOP)) {
Variant value;
int err = fb_compact_unserialize_from_buffer(value, buf, n, p);
int err =
fb_compact_unserialize_from_buffer(value, buf, n, p, depth + 1);
if (err) {
return err;
}
Expand All @@ -799,7 +804,8 @@ int fb_compact_unserialize_from_buffer(
++p;
} else {
Variant value;
int err = fb_compact_unserialize_from_buffer(value, buf, n, p);
int err =
fb_compact_unserialize_from_buffer(value, buf, n, p, depth + 1);
if (err) {
return err;
}
Expand All @@ -820,12 +826,12 @@ int fb_compact_unserialize_from_buffer(
Array arr = Array::CreateDArray();
while (p < n && buf[p] != (char)(kCodePrefix | FB_CS_STOP)) {
Variant key;
int err = fb_compact_unserialize_from_buffer(key, buf, n, p);
int err = fb_compact_unserialize_from_buffer(key, buf, n, p, depth + 1);
if (err) {
return err;
}
Variant value;
err = fb_compact_unserialize_from_buffer(value, buf, n, p);
err = fb_compact_unserialize_from_buffer(value, buf, n, p, depth + 1);
if (err) {
return err;
}
Expand Down Expand Up @@ -861,7 +867,7 @@ Variant fb_compact_unserialize(const char* str, int len,

Variant ret;
int p = 0;
int err = fb_compact_unserialize_from_buffer(ret, str, len, p);
int err = fb_compact_unserialize_from_buffer(ret, str, len, p, 0);
if (err) {
success = false;
errcode = err;
Expand Down Expand Up @@ -1313,6 +1319,7 @@ struct FBExtension : Extension {
HHVM_RC_INT_SAME(FB_UNSERIALIZE_UNEXPECTED_END);
HHVM_RC_INT_SAME(FB_UNSERIALIZE_UNRECOGNIZED_OBJECT_TYPE);
HHVM_RC_INT_SAME(FB_UNSERIALIZE_UNEXPECTED_ARRAY_KEY_TYPE);
HHVM_RC_INT_SAME(FB_UNSERIALIZE_MAX_DEPTH_EXCEEDED);

HHVM_RC_INT(FB_SERIALIZE_HACK_ARRAYS, k_FB_SERIALIZE_HACK_ARRAYS);
HHVM_RC_INT(FB_SERIALIZE_VARRAY_DARRAY, k_FB_SERIALIZE_VARRAY_DARRAY);
Expand Down
9 changes: 9 additions & 0 deletions hphp/test/slow/ext_fb/unserialize-bad-set.php
@@ -0,0 +1,9 @@
<?hh
// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.

<<__EntryPoint>>
function main() {
$ret = null;
var_dump(fb_unserialize("\x14\x02\x01\x14\x02\x01\x02\x01\x01\x01", inout $ret));
var_dump($ret);
}
2 changes: 2 additions & 0 deletions hphp/test/slow/ext_fb/unserialize-bad-set.php.expect
@@ -0,0 +1,2 @@
bool(false)
bool(false)
53 changes: 53 additions & 0 deletions hphp/test/slow/ext_fb/unserialize_max_depth.php
@@ -0,0 +1,53 @@
<?hh
// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.

function make_fb_serialize_struct($size) {
return str_repeat("\x0a\x02\x01", $size) . "\x02\x01" . str_repeat("\x01", $size);
}

function make_fb_serialize_vector($size) {
return str_repeat("\x12", $size) . str_repeat("\x01", $size);
}

function make_fb_serialize_list($size) {
return str_repeat("\x13\x02\x01", $size) . "\x02\x01" . str_repeat("\x01", $size);
}

function make_fb_cs_list_map($size) {
return str_repeat("\xfa\xfd", $size) . str_repeat("\xfc", $size);
}

function make_fb_cs_map($size) {
return str_repeat("\xfb\x01", $size) . "\x01" . str_repeat("\xfc", $size);
}

function make_fb_cs_vector($size) {
return str_repeat("\xfe", $size) . str_repeat("\xfc", $size);
}

function test($serialized) {
$ret = null;
var_dump(
fb_unserialize(
$serialized,
inout $ret,
FB_SERIALIZE_HACK_ARRAYS
)
);
var_dump($ret);
}

function tests($size) {
test(make_fb_serialize_struct($size));
test(make_fb_serialize_vector($size));
test(make_fb_serialize_list($size));
test(make_fb_cs_list_map($size));
test(make_fb_cs_map($size));
test(make_fb_cs_vector($size));
}

<<__EntryPoint>>
function main() {
tests(10);
tests(1026);
}

0 comments on commit 1746dfb

Please sign in to comment.