Rooba
/
flatbuffers
mirror of https://github.com/google/flatbuffers.git
1
1
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[idl_parser] Improve stack overflow protection (#6364) 

* [idl_parser] Improve stack overflow protection

Add stack overflow protection for Flexbuffer and nested Flatbuffer parsers.
Replaces the `Recurse()` method by the new ParseDepthGuard RAII class.

* Remove move operator from Parser.

It was wrong decision to add move ctor and assignment into Parser class.
These operators will make it extremely difficult to add constant or reference fields in the future.

* Remove ';' from definition of FLATBUFFERS_DELETE_FUNC

* Format code

* Make this PR compatible with MSVC2010 (it doesn't support inherited ctor)
This commit is contained in:
Vladimir Glavnyy 2021-01-05 03:39:12 +07:00 committed by GitHub
parent e7430bbebd
commit 82836a62be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 84 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/flatbuffers/base.h
View File

@ -197,9 +197,9 @@ namespace flatbuffers {
#if (!defined(_MSC_VER) || _MSC_FULL_VER >= 180020827) && \
(!defined(__GNUC__) || (__GNUC__ * 100 + __GNUC_MINOR__ >= 404)) || \
defined(__clang__)
#define FLATBUFFERS_DELETE_FUNC(func) func = delete;
#define FLATBUFFERS_DELETE_FUNC(func) func = delete
#else
#define FLATBUFFERS_DELETE_FUNC(func) private: func;
#define FLATBUFFERS_DELETE_FUNC(func) private: func
#endif
#if (!defined(_MSC_VER) || _MSC_VER >= 1900) && \

13
include/flatbuffers/flatbuffers.h
View File

@ -821,9 +821,9 @@ class DetachedBuffer {
#if !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
// These may change access mode, leave these at end of public section
FLATBUFFERS_DELETE_FUNC(DetachedBuffer(const DetachedBuffer &other))
FLATBUFFERS_DELETE_FUNC(DetachedBuffer(const DetachedBuffer &other));
FLATBUFFERS_DELETE_FUNC(
DetachedBuffer &operator=(const DetachedBuffer &other))
DetachedBuffer &operator=(const DetachedBuffer &other));
// clang-format off
#endif // !defined(FLATBUFFERS_CPP98_STL)
// clang-format on
@ -1066,8 +1066,8 @@ class vector_downward {
private:
// You shouldn't really be copying instances of this class.
FLATBUFFERS_DELETE_FUNC(vector_downward(const vector_downward &))
FLATBUFFERS_DELETE_FUNC(vector_downward &operator=(const vector_downward &))
FLATBUFFERS_DELETE_FUNC(vector_downward(const vector_downward &));
FLATBUFFERS_DELETE_FUNC(vector_downward &operator=(const vector_downward &));
Allocator *allocator_;
bool own_allocator_;
@ -1891,7 +1891,7 @@ class FlatBufferBuilder {
}
FLATBUFFERS_DELETE_FUNC(
StructKeyComparator &operator=(const StructKeyComparator &))
StructKeyComparator &operator=(const StructKeyComparator &));
};
/// @endcond
@ -1966,7 +1966,8 @@ class FlatBufferBuilder {
vector_downward &buf_;
private:
FLATBUFFERS_DELETE_FUNC(TableKeyComparator &operator=(const TableKeyComparator &other))
FLATBUFFERS_DELETE_FUNC(
TableKeyComparator &operator=(const TableKeyComparator &other));
};
/// @endcond

17
include/flatbuffers/idl.h
View File

@ -35,7 +35,7 @@
// Definition Language) / schema file.
// Limits maximum depth of nested objects.
// Prevents stack overflow while parse flatbuffers or json.
// Prevents stack overflow while parse scheme, or json, or flexbuffer.
#if !defined(FLATBUFFERS_MAX_PARSING_DEPTH)
# define FLATBUFFERS_MAX_PARSING_DEPTH 64
#endif
@ -767,8 +767,8 @@ class Parser : public ParserState {
opts(options),
uses_flexbuffers_(false),
source_(nullptr),
anonymous_counter(0),
recurse_protection_counter(0) {
anonymous_counter_(0),
parse_depth_counter_(0) {
if (opts.force_defaults) { builder_.ForceDefaults(true); }
// Start out with the empty namespace being current.
empty_namespace_ = new Namespace();
@ -806,11 +806,6 @@ class Parser : public ParserState {
}
}
#ifdef FLATBUFFERS_DEFAULT_DECLARATION
Parser(Parser&&) = default;
Parser& operator=(Parser&&) = default;
#endif
// Parse the string containing either schema or JSON data, which will
// populate the SymbolTable's or the FlatBufferBuilder above.
// include_paths is used to resolve any include statements, and typically
@ -872,6 +867,8 @@ class Parser : public ParserState {
static bool SupportsOptionalScalars(const flatbuffers::IDLOptions &opts);
private:
class ParseDepthGuard;
void Message(const std::string &msg);
void Warning(const std::string &msg);
FLATBUFFERS_CHECKED_ERROR ParseHexNum(int nibbles, uint64_t *val);
@ -1000,8 +997,8 @@ class Parser : public ParserState {
std::vector<std::pair<Value, FieldDef *>> field_stack_;
int anonymous_counter;
int recurse_protection_counter;
int anonymous_counter_;
int parse_depth_counter_; // stack-overflow guard
};
// Utility functions for multiple generators:

86
src/idl_parser.cpp
View File

@ -155,19 +155,35 @@ CheckedError Parser::Error(const std::string &msg) {
inline CheckedError NoError() { return CheckedError(false); }
CheckedError Parser::RecurseError() {
return Error("maximum parsing recursion of " +
NumToString(FLATBUFFERS_MAX_PARSING_DEPTH) + " reached");
return Error("maximum parsing depth " + NumToString(parse_depth_counter_) +
" reached");
}
template<typename F> CheckedError Parser::Recurse(F f) {
if (recurse_protection_counter >= (FLATBUFFERS_MAX_PARSING_DEPTH))
return RecurseError();
recurse_protection_counter++;
auto ce = f();
recurse_protection_counter--;
return ce;
class Parser::ParseDepthGuard {
public:
explicit ParseDepthGuard(Parser *parser_not_null)
: parser_(*parser_not_null), caller_depth_(parser_.parse_depth_counter_) {
FLATBUFFERS_ASSERT(caller_depth_ <= (FLATBUFFERS_MAX_PARSING_DEPTH) &&
"Check() must be called to prevent stack overflow");
parser_.parse_depth_counter_ += 1;
}
~ParseDepthGuard() { parser_.parse_depth_counter_ -= 1; }
CheckedError Check() {
return caller_depth_ >= (FLATBUFFERS_MAX_PARSING_DEPTH)
? parser_.RecurseError()
: CheckedError(false);
}
FLATBUFFERS_DELETE_FUNC(ParseDepthGuard(const ParseDepthGuard &));
FLATBUFFERS_DELETE_FUNC(ParseDepthGuard &operator=(const ParseDepthGuard &));
private:
Parser &parser_;
const int caller_depth_;
};
template<typename T> std::string TypeToIntervalString() {
return "[" + NumToString((flatbuffers::numeric_limits<T>::lowest)()) + "; " +
NumToString((flatbuffers::numeric_limits<T>::max)()) + "]";
@ -633,9 +649,11 @@ CheckedError Parser::ParseType(Type &type) {
ECHECK(ParseTypeIdent(type));
}
} else if (token_ == '[') {
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
NEXT();
Type subtype;
ECHECK(Recurse([&]() { return ParseType(subtype); }));
ECHECK(ParseType(subtype));
if (IsSeries(subtype)) {
// We could support this, but it will complicate things, and it's
// easier to work around with a struct around the inner vector.
@ -1033,6 +1051,8 @@ CheckedError Parser::ParseAnyValue(Value &val, FieldDef *field,
}
if (next_name == type_name) {
EXPECT(':');
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
Value type_val = type_field->value;
ECHECK(ParseAnyValue(type_val, type_field, 0, nullptr, 0));
constant = type_val.constant;
@ -1159,6 +1179,9 @@ CheckedError Parser::ParseTableDelimiters(size_t &fieldn,
CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value,
uoffset_t *ovalue) {
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
size_t fieldn_outer = 0;
auto err = ParseTableDelimiters(
fieldn_outer, &struct_def,
@ -1194,9 +1217,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value,
ECHECK(
ParseNestedFlatbuffer(val, field, fieldn, struct_def_inner));
} else {
ECHECK(Recurse([&]() {
return ParseAnyValue(val, field, fieldn, struct_def_inner, 0);
}));
ECHECK(ParseAnyValue(val, field, fieldn, struct_def_inner, 0));
}
// Hardcoded insertion-sort with error-check.
// If fields are specified in order, then this loop exits
@ -1372,9 +1393,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue,
auto err = ParseVectorDelimiters(count, [&](uoffset_t &) -> CheckedError {
Value val;
val.type = type;
ECHECK(Recurse([&]() {
return ParseAnyValue(val, field, fieldn, nullptr, count, true);
}));
ECHECK(ParseAnyValue(val, field, fieldn, nullptr, count, true));
field_stack_.push_back(std::make_pair(val, nullptr));
return NoError();
});
@ -1548,7 +1567,7 @@ CheckedError Parser::ParseNestedFlatbuffer(Value &val, FieldDef *field,
nested_parser.enums_ = enums_;
nested_parser.opts = opts;
nested_parser.uses_flexbuffers_ = uses_flexbuffers_;
nested_parser.parse_depth_counter_ = parse_depth_counter_;
// Parse JSON substring into new flatbuffer builder using nested_parser
bool ok = nested_parser.Parse(substring.c_str(), nullptr, nullptr);
@ -1697,6 +1716,9 @@ static inline void SingleValueRepack(Value &e, double val) {
#endif
CheckedError Parser::ParseFunction(const std::string *name, Value &e) {
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
// Copy name, attribute will be changed on NEXT().
const auto functionname = attribute_;
if (!IsFloat(e.type.base_type)) {
@ -1707,7 +1729,7 @@ CheckedError Parser::ParseFunction(const std::string *name, Value &e) {
}
NEXT();
EXPECT('(');
ECHECK(Recurse([&]() { return ParseSingleValue(name, e, false); }));
ECHECK(ParseSingleValue(name, e, false));
EXPECT(')');
// calculate with double precision
double x, y = 0.0;
@ -2678,7 +2700,7 @@ CheckedError Parser::ParseProtoFields(StructDef *struct_def, bool isextend,
ECHECK(StartEnum(name, true, &oneof_union));
type = Type(BASE_TYPE_UNION, nullptr, oneof_union);
} else {
auto name = "Anonymous" + NumToString(anonymous_counter++);
auto name = "Anonymous" + NumToString(anonymous_counter_++);
ECHECK(StartStruct(name, &anonymous_struct));
type = Type(BASE_TYPE_STRUCT, anonymous_struct);
}
@ -2851,14 +2873,16 @@ CheckedError Parser::ParseTypeFromProtoType(Type *type) {
}
CheckedError Parser::SkipAnyJsonValue() {
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
switch (token_) {
case '{': {
size_t fieldn_outer = 0;
return ParseTableDelimiters(
fieldn_outer, nullptr,
return ParseTableDelimiters(fieldn_outer, nullptr,
[&](const std::string &, size_t &fieldn,
const StructDef *) -> CheckedError {
ECHECK(Recurse([&]() { return SkipAnyJsonValue(); }));
ECHECK(SkipAnyJsonValue());
fieldn++;
return NoError();
});
@ -2866,7 +2890,7 @@ CheckedError Parser::SkipAnyJsonValue() {
case '[': {
uoffset_t count = 0;
return ParseVectorDelimiters(count, [&](uoffset_t &) -> CheckedError {
return Recurse([&]() { return SkipAnyJsonValue(); });
return SkipAnyJsonValue();
});
}
case kTokenStringConstant:
@ -2882,6 +2906,9 @@ CheckedError Parser::SkipAnyJsonValue() {
}
CheckedError Parser::ParseFlexBufferValue(flexbuffers::Builder *builder) {
ParseDepthGuard depth_guard(this);
ECHECK(depth_guard.Check());
switch (token_) {
case '{': {
auto start = builder->StartMap();
@ -2943,15 +2970,19 @@ CheckedError Parser::ParseFlexBufferValue(flexbuffers::Builder *builder) {
bool Parser::ParseFlexBuffer(const char *source, const char *source_filename,
flexbuffers::Builder *builder) {
const auto initial_depth = parse_depth_counter_;
(void)initial_depth;
auto ok = !StartParseFile(source, source_filename).Check() &&
!ParseFlexBufferValue(builder).Check();
if (ok) builder->Finish();
FLATBUFFERS_ASSERT(initial_depth == parse_depth_counter_);
return ok;
}
bool Parser::Parse(const char *source, const char **include_paths,
const char *source_filename) {
FLATBUFFERS_ASSERT(0 == recurse_protection_counter);
const auto initial_depth = parse_depth_counter_;
(void)initial_depth;
bool r;
if (opts.use_flexbuffers) {
@ -2959,16 +2990,17 @@ bool Parser::Parse(const char *source, const char **include_paths,
} else {
r = !ParseRoot(source, include_paths, source_filename).Check();
}
FLATBUFFERS_ASSERT(0 == recurse_protection_counter);
FLATBUFFERS_ASSERT(initial_depth == parse_depth_counter_);
return r;
}
bool Parser::ParseJson(const char *json, const char *json_filename) {
FLATBUFFERS_ASSERT(0 == recurse_protection_counter);
const auto initial_depth = parse_depth_counter_;
(void)initial_depth;
builder_.Clear();
const auto done =
!StartParseFile(json, json_filename).Check() && !DoParseJson().Check();
FLATBUFFERS_ASSERT(0 == recurse_protection_counter);
FLATBUFFERS_ASSERT(initial_depth == parse_depth_counter_);
return done;
}

2
tests/fuzzer/CMakeLists.txt
View File

@ -10,7 +10,7 @@ option(BUILD_DEBUGGER "Compile a debugger with main() and without libFuzzer" OFF
if(NOT DEFINED FLATBUFFERS_MAX_PARSING_DEPTH)
# Force checking of RecursionError in the test
set(FLATBUFFERS_MAX_PARSING_DEPTH 8)
set(FLATBUFFERS_MAX_PARSING_DEPTH 24)
endif()
message(STATUS "FLATBUFFERS_MAX_PARSING_DEPTH: ${FLATBUFFERS_MAX_PARSING_DEPTH}")

8
tests/fuzzer/flatbuffers_monster_fuzzer.cc
View File

@ -45,7 +45,8 @@ std::string LoadBinarySchema(const char *file_name) {
return schemafile;
}
flatbuffers::Parser make_parser(const flatbuffers::IDLOptions opts) {
std::string do_test(const flatbuffers::IDLOptions &opts,
const std::string input_json) {
// once loaded from disk
static const std::string schemafile = LoadBinarySchema("./monster_test.bfbs");
// parse schema first, so we can use it to parse the data after
@ -55,12 +56,7 @@ flatbuffers::Parser make_parser(const flatbuffers::IDLOptions opts) {
schemafile.size()));
// (re)define parser options
parser.opts = opts;
return parser;
}
std::string do_test(const flatbuffers::IDLOptions &opts,
const std::string input_json) {
auto parser = make_parser(opts);
std::string jsongen;
if (parser.ParseJson(input_json.c_str())) {
flatbuffers::Verifier verifier(parser.builder_.GetBufferPointer(),