Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix[bmqeval]: limit expression length to avoid stack overflow #441

Merged
merged 1 commit into from
Oct 4, 2024
+121 −66
Merged

Fix[bmqeval]: limit expression length to avoid stack overflow #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/groups/bmq/bmqeval/bmqeval_simpleevaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ void SimpleEvaluator::parse(const bsl::string& expression,
context.d_numProperties = 0;
context.d_os.reset();

if (expression.length() > k_MAX_EXPRESSION_LENGTH) {
context.d_os << "expression is too long (" << expression.length()
<< "), max allowed length: " << k_MAX_EXPRESSION_LENGTH;
context.d_lastError = ErrorType::e_TOO_LONG;
return; // RETURN
}

bsl::istringstream is(expression, context.d_allocator);
SimpleEvaluatorScanner scanner;
scanner.switch_streams(&is, &context.d_os);
Expand All @@ -90,28 +97,27 @@ void SimpleEvaluator::parse(const bsl::string& expression,

if (parser.parse() != 0) {
context.d_lastError = ErrorType::e_SYNTAX;

return; // RETURN
}

if (context.d_numOperators > k_MAX_OPERATORS) {
context.d_os << "too many operators";
context.d_os << "too many operators (" << context.d_numOperators
<< "), max allowed operators: " << k_MAX_OPERATORS;
context.d_lastError = ErrorType::e_TOO_COMPLEX;

return; // RETURN
}

if (context.d_numProperties == 0) {
context.d_os << "expression does not use any properties";
context.d_lastError = ErrorType::e_NO_PROPERTIES;

return; // RETURN
}

if (context.d_numProperties > k_MAX_PROPERTIES) {
context.d_os << "expression uses too many properties";
context.d_os << "expression uses too many properties ("
<< context.d_numProperties
<< "), max allowed properties: " << k_MAX_PROPERTIES;
context.d_lastError = ErrorType::e_TOO_COMPLEX;

return; // RETURN
}
}
Expand Down Expand Up @@ -204,8 +210,8 @@ SimpleEvaluator::IntegerLiteral::evaluate(EvaluationContext& context) const
// class SimpleEvaluator::BooleanLiteral
// -------------------------------------

bdld::Datum
SimpleEvaluator::BooleanLiteral::evaluate(EvaluationContext& context) const
bdld::Datum SimpleEvaluator::BooleanLiteral::evaluate(
BSLS_ANNOTATION_UNUSED EvaluationContext& context) const
{
return bdld::Datum::createBoolean(d_value);
}
Expand Down
71 changes: 50 additions & 21 deletions src/groups/bmq/bmqeval/bmqeval_simpleevaluator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,25 @@ class EvaluationContext;

struct ErrorType {
enum Enum {
e_OK = 0 // no error
,
/// no error
e_OK = 0,

e_COMPILATION_FIRST = -100,
e_SYNTAX = -100 // syntax error
,
e_NO_PROPERTIES = -101 // expression does not use any property
,
e_TOO_COMPLEX = -102 // too many properties
// or operators in expression
,
e_COMPILATION_LAST = -102,

/// syntax error
e_SYNTAX = -100,

/// expression does not use any property
e_NO_PROPERTIES = -101,

/// too many properties or operators in expression
e_TOO_COMPLEX = -102,

/// expression string is too long
e_TOO_LONG = -103,

e_COMPILATION_LAST = -103,

e_EVALUATION_FIRST = -1,
e_NAME = -1,
e_TYPE = -2,
Expand Down Expand Up @@ -492,10 +500,14 @@ class SimpleEvaluator {
public:
// PUBLIC CONSTANTS
enum {
/// The maximum length of an expression string.
k_MAX_EXPRESSION_LENGTH = 128,

/// The maximum number of operators allowed in a single expression.
k_MAX_OPERATORS = 10,
k_MAX_OPERATORS = 10,

/// The maximum number of properties allowed in a single expression.
k_MAX_PROPERTIES = 10
// The maximum number of properties allowed in a single expression.
};

// CREATORS
Expand Down Expand Up @@ -706,19 +718,36 @@ class EvaluationContext {
inline const char* ErrorType::toString(ErrorType::Enum value)
{
switch (value) {
case 0: return ""; // RETURN
case bmqeval::ErrorType::e_SYNTAX: return "syntax error"; // RETURN
case bmqeval::ErrorType::e_TOO_COMPLEX:
return "subscription expression is too complex"; // RETURN
case bmqeval::ErrorType::e_NO_PROPERTIES:
case bmqeval::ErrorType::e_OK: {
return ""; // RETURN
}
case bmqeval::ErrorType::e_SYNTAX: {
return "syntax error"; // RETURN
}
case bmqeval::ErrorType::e_NO_PROPERTIES: {
return "expression does not use any property"; // RETURN
case bmqeval::ErrorType::e_NAME:
}
case bmqeval::ErrorType::e_TOO_COMPLEX: {
return "subscription expression is too complex"; // RETURN
}
case bmqeval::ErrorType::e_TOO_LONG: {
return "subscription expression is too long"; // RETURN
}
case bmqeval::ErrorType::e_NAME: {
return "undefined name in subscription expression"; // RETURN
case bmqeval::ErrorType::e_TYPE:
}
case bmqeval::ErrorType::e_TYPE: {
return "type error in expression"; // RETURN
case bmqeval::ErrorType::e_BINARY:
}
case bmqeval::ErrorType::e_BINARY: {
return "binary properties are not supported"; // RETURN
default: return "unknown error"; // RETURN
}
case bmqeval::ErrorType::e_UNDEFINED: {
return "undefined error"; // RETURN
}
default: {
return "unknown error"; // RETURN
}
}
}

Expand Down
92 changes: 56 additions & 36 deletions src/groups/bmq/bmqeval/bmqeval_simpleevaluator.t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ class MockPropertiesReader : public PropertiesReader {
// MANIPULATORS

/// Return a `bdld::Datum` object with value for the specified `name`.
/// Use the specified `allocator` for any memory allocation.
virtual bdld::Datum get(const bsl::string& name,
bslma::Allocator* allocator)
/// The `bslma::Allocator*` argument is unused.
bdld::Datum get(const bsl::string& name,
bslma::Allocator*) BSLS_KEYWORD_OVERRIDE
{
bsl::unordered_map<bsl::string, bdld::Datum>::const_iterator iter =
d_map.find(name);
Expand Down Expand Up @@ -124,6 +124,30 @@ static bsl::string makeTooManyOperators()
return os.str();
}

static bsl::string makeTooLongExpression()
{
mwcu::MemOutStream os(s_allocator_p);

// Note that we want to create `k_STACK_SIZE` nested NOT objects in AST,
// and when we call destructor chain for all these objects, we'll need
// much more memory on a stack, since each destructor address is not a
// single byte, and also we need to call shared_ptr and Not destructors in
// pairs. But the initial guess of `k_STACK_SIZE` is more than sufficient
// to cause segfault if this case is not handled properly.
const size_t k_STACK_SIZE = 1024 * 1024;
ASSERT(SimpleEvaluator::k_MAX_EXPRESSION_LENGTH < k_STACK_SIZE);

for (size_t i = 0; i < k_STACK_SIZE; i += 16) {
// Combining `!` and `~` differently in case we want to introduce
// "NOT" optimization one day (remove double sequential "NOTs")
os << "!!!!~~~~!~!~!!~~";
}

os << "x";

return os.str();
}

static void test1_compilationErrors()
{
struct TestParameters {
Expand All @@ -139,10 +163,13 @@ static void test1_compilationErrors()
"syntax error, unexpected invalid character at offset 3"},
{makeTooManyOperators(),
ErrorType::e_TOO_COMPLEX,
"too many operators"},
"too many operators (12), max allowed operators: 10"},
{"true && true",
ErrorType::e_NO_PROPERTIES,
"expression does not use any properties"},
{makeTooLongExpression(),
ErrorType::e_TOO_LONG,
"expression is too long (1048577), max allowed length: 128"},

// only C-style operator variants supported
{"val = 42",
Expand All @@ -157,6 +184,27 @@ static void test1_compilationErrors()
{"val <> 42",
ErrorType::e_SYNTAX,
"syntax error, unexpected > at offset 5"},

// unsupported_ints
{"i_0 != 9223372036854775808",
ErrorType::e_SYNTAX,
"integer overflow at offset 7"}, // 2 ** 63
{"i_0 != -170141183460469231731687303715884105728",
ErrorType::e_SYNTAX,
"integer overflow at offset 7"}, // -(2 ** 127)
{"i_0 != 170141183460469231731687303715884105727",
ErrorType::e_SYNTAX,
"integer overflow at offset 7"}, // 2 ** 127 - 1
{"i_0 != "
"-5789604461865809771178549250434395392663499233282028201972879200395"
"6564819968",
ErrorType::e_SYNTAX,
"integer overflow at offset 7"}, // -(2 ** 255)
{"i_0 != "
"57896044618658097711785492504343953926634992332820282019728792003956"
"564819967",
ErrorType::e_SYNTAX,
"integer overflow at offset 7"}, // 2 ** 255 - 1
};
const TestParameters* testParametersEnd = testParameters +
sizeof(testParameters) /
Expand Down Expand Up @@ -287,7 +335,6 @@ static void test3_evaluation()
struct TestParameters {
const char* expression;
bool expected;
bool expectCompilationFailure;
} testParameters[] = {
{"b_true", true},

Expand Down Expand Up @@ -427,25 +474,6 @@ static void test3_evaluation()
{"i_0 != -9223372036854775807", true}, // -(2 ** 63) + 1
{"i_0 != 9223372036854775807", true}, // 2 ** 63 - 1
{"i_0 != -9223372036854775808", true}, // -(2 ** 63)

// unsupported_ints
{"i_0 != 9223372036854775808", false, true}, // 2 ** 63
{"i_0 != -170141183460469231731687303715884105728",
false,
true}, // -(2 ** 127)
{"i_0 != 170141183460469231731687303715884105727",
false,
true}, // 2 ** 127 - 1
{"i_0 != "
"-5789604461865809771178549250434395392663499233282028201972879200395"
"6564819968",
false,
true}, // -(2 ** 255)
{"i_0 != "
"57896044618658097711785492504343953926634992332820282019728792003956"
"564819967",
false,
true}, // 2 ** 255 - 1
};
const TestParameters* testParametersEnd = testParameters +
sizeof(testParameters) /
Expand All @@ -461,20 +489,12 @@ static void test3_evaluation()

ASSERT(!evaluator.isValid());

if (int rc = evaluator.compile(parameters->expression,
compilationContext)) {
if (!parameters->expectCompilationFailure) {
PV(bsl::string("UNEXPECTED: ") +
compilationContext.lastErrorMessage());
ASSERT(false);
}
if (evaluator.compile(parameters->expression, compilationContext)) {
PV(bsl::string("UNEXPECTED: ") +
compilationContext.lastErrorMessage());
ASSERT(false);
}
else {
if (parameters->expectCompilationFailure) {
PV("Expected compilation failure");
ASSERT(false);
}

ASSERT(evaluator.isValid());
ASSERT_EQ(evaluator.evaluate(evaluationContext),
parameters->expected);
Expand Down
2 changes: 1 addition & 1 deletion src/groups/bmq/bmqeval/bmqeval_simpleevaluatorparser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ expression
{ $$ = ctx.makeLiteral<SimpleEvaluator::IntegerLiteral>($1); }
| OVERFLOW
{
ctx.d_os << " integer overflow at offset " << scanner.lastTokenLocation();
ctx.d_os << "integer overflow at offset " << scanner.lastTokenLocation();
YYABORT;
}
| STRING
Expand Down
Loading