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.

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

Implement nesting guard to avoid "out of stack space" #2438

Merged
merged 1 commit into from
Jul 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions src/ast_def_macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ class LocalOption {
};

#define LOCAL_FLAG(name,opt) LocalOption<bool> flag_##name(name, opt)
#define LOCAL_COUNT(name,opt) LocalOption<size_t> cnt_##name(name, opt)

#define NESTING_GUARD(name) \
LocalOption<size_t> cnt_##name(name, name + 1); \
if (nestings > MAX_NESTING) throw Exception::NestingLimitError(pstate); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should nestings be name here?

Copy link
Contributor Author

@mgreter mgreter Oct 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the Macro is called as NESTING_GUARD(nestings), therefore it will in turn insert a call to LocalOption<size_t> cnt_nestings(nestings, nestings + 1). Meaning it will store the previous value and then increase it by one (reverting once local variable cnt_nesting goes out of scope).

Sorry, read your statement the reverse way. Yeah, the second line should be name I guess.


#define ATTACH_OPERATIONS()\
virtual void perform(Operation<void>* op) { (*op)(this); }\
Expand Down
4 changes: 4 additions & 0 deletions src/error_handling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ namespace Sass {
: Base(pstate, msg, import_stack)
{ }

NestingLimitError::NestingLimitError(ParserState pstate, std::string msg, std::vector<Sass_Import_Entry>* import_stack)
: Base(pstate, msg, import_stack)
{ }

UndefinedOperation::UndefinedOperation(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, const std::string& op)
: lhs(lhs), rhs(rhs), op(op)
{
Expand Down
7 changes: 7 additions & 0 deletions src/error_handling.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Sass {
const std::string def_msg = "Invalid sass detected";
const std::string def_op_msg = "Undefined operation";
const std::string def_op_null_msg = "Invalid null operation";
const std::string def_nesting_limit = "Code too deeply neested";

class Base : public std::runtime_error {
protected:
Expand Down Expand Up @@ -83,6 +84,12 @@ namespace Sass {
virtual ~InvalidSyntax() throw() {};
};

class NestingLimitError : public Base {
public:
NestingLimitError(ParserState pstate, std::string msg = def_nesting_limit, std::vector<Sass_Import_Entry>* import_stack = 0);
virtual ~NestingLimitError() throw() {};
};

/* common virtual base class (has no pstate) */
class OperationError : public std::runtime_error {
protected:
Expand Down
15 changes: 15 additions & 0 deletions src/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ namespace Sass {
// a ruleset connects a selector and a block
Ruleset_Obj Parser::parse_ruleset(Lookahead lookahead)
{
NESTING_GUARD(nestings);
// inherit is_root from parent block
Block_Obj parent = block_stack.back();
bool is_root = parent && parent->is_root();
Expand Down Expand Up @@ -535,6 +536,7 @@ namespace Sass {
// in the eval stage we will be re-parse it into an actual selector
Selector_Schema_Obj Parser::parse_selector_schema(const char* end_of_selector, bool chroot)
{
NESTING_GUARD(nestings);
// move up to the start
lex< optional_spaces >();
const char* i = position;
Expand Down Expand Up @@ -646,6 +648,7 @@ namespace Sass {
{
bool reloop;
bool had_linefeed = false;
NESTING_GUARD(nestings);
Complex_Selector_Obj sel;
Selector_List_Obj group = SASS_MEMORY_NEW(Selector_List, pstate);
group->media_block(last_media_block);
Expand Down Expand Up @@ -700,6 +703,7 @@ namespace Sass {
Complex_Selector_Obj Parser::parse_complex_selector(bool chroot)
{

NESTING_GUARD(nestings);
String_Obj reference = 0;
lex < block_comment >();
advanceToNextToken();
Expand Down Expand Up @@ -1055,6 +1059,7 @@ namespace Sass {

Expression_Obj Parser::parse_map()
{
NESTING_GUARD(nestings);
Expression_Obj key = parse_list();
List_Obj map = SASS_MEMORY_NEW(List, pstate, 0, SASS_HASH);

Expand Down Expand Up @@ -1098,6 +1103,7 @@ namespace Sass {

Expression_Obj Parser::parse_bracket_list()
{
NESTING_GUARD(nestings);
// check if we have an empty list
// return the empty list as such
if (peek_css< list_terminator >(position))
Expand Down Expand Up @@ -1144,12 +1150,14 @@ namespace Sass {
// so to speak: we unwrap items from lists if possible here!
Expression_Obj Parser::parse_list(bool delayed)
{
NESTING_GUARD(nestings);
return parse_comma_list(delayed);
}

// will return singletons unwrapped
Expression_Obj Parser::parse_comma_list(bool delayed)
{
NESTING_GUARD(nestings);
// check if we have an empty list
// return the empty list as such
if (peek_css< list_terminator >(position))
Expand Down Expand Up @@ -1189,6 +1197,7 @@ namespace Sass {
// will return singletons unwrapped
Expression_Obj Parser::parse_space_list()
{
NESTING_GUARD(nestings);
Expression_Obj disj1 = parse_disjunction();
// if it's a singleton, return it (don't wrap it)
if (peek_css< space_list_terminator >(position)
Expand All @@ -1213,6 +1222,7 @@ namespace Sass {
// parse logical OR operation
Expression_Obj Parser::parse_disjunction()
{
NESTING_GUARD(nestings);
advanceToNextToken();
ParserState state(pstate);
// parse the left hand side conjunction
Expand All @@ -1234,6 +1244,7 @@ namespace Sass {
// parse logical AND operation
Expression_Obj Parser::parse_conjunction()
{
NESTING_GUARD(nestings);
advanceToNextToken();
ParserState state(pstate);
// parse the left hand side relation
Expand All @@ -1256,6 +1267,7 @@ namespace Sass {
// parse comparison operations
Expression_Obj Parser::parse_relation()
{
NESTING_GUARD(nestings);
advanceToNextToken();
ParserState state(pstate);
// parse the left hand side expression
Expand Down Expand Up @@ -1308,6 +1320,7 @@ namespace Sass {
// parse addition and subtraction operations
Expression_Obj Parser::parse_expression()
{
NESTING_GUARD(nestings);
advanceToNextToken();
ParserState state(pstate);
// parses multiple add and subtract operations
Expand Down Expand Up @@ -1351,6 +1364,7 @@ namespace Sass {
// parse addition and subtraction operations
Expression_Obj Parser::parse_operators()
{
NESTING_GUARD(nestings);
advanceToNextToken();
ParserState state(pstate);
Expression_Obj factor = parse_factor();
Expand Down Expand Up @@ -1383,6 +1397,7 @@ namespace Sass {
// called from parse_value_schema
Expression_Obj Parser::parse_factor()
{
NESTING_GUARD(nestings);
lex < css_comments >(false);
if (lex_css< exactly<'('> >()) {
// parse_map may return a list
Expand Down
14 changes: 12 additions & 2 deletions src/parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@
#include "position.hpp"
#include "prelexer.hpp"

#ifndef MAX_NESTING
// Note that this limit is not an exact science
// it depends on various factors, which some are
// not under our control (compile time or even OS
// dependent settings on the available stack size)
// It should fix most common segfault cases though.
#define MAX_NESTING 512
#endif

struct Lookahead {
const char* found;
const char* error;
Expand All @@ -35,13 +44,14 @@ namespace Sass {
Position before_token;
Position after_token;
ParserState pstate;
int indentation;
size_t indentation;
size_t nestings;

Token lexed;

Parser(Context& ctx, const ParserState& pstate)
: ParserState(pstate), ctx(ctx), block_stack(), stack(0), last_media_block(),
source(0), position(0), end(0), before_token(pstate), after_token(pstate), pstate(pstate), indentation(0)
source(0), position(0), end(0), before_token(pstate), after_token(pstate), pstate(pstate), indentation(0), nestings(0)
{ stack.push_back(Scope::Root); }

// static Parser from_string(const std::string& src, Context& ctx, ParserState pstate = ParserState("[STRING]"));
Expand Down