Prevent stack overflow in macro expansion
Add a configurable limit for how many nested MacroExpander objects can
be created in the preprocessor, so that stack overflow can be
prevented in case of malicious shaders. By default the limit is set to
1000. In unit tests the limit is set lower to make the test run
faster.
Includes refactoring of most of the preprocessor tests so that they
use utility functions provided by the test class instead of repeating
the same code for initializing the preprocessor.
BUG=angleproject:1600
TEST=angle_unittests
Change-Id: I23b5140d9f2dc52df96111650db63150f7238494
Reviewed-on: https://chromium-review.googlesource.com/413986
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/preprocessor/DiagnosticsBase.cpp b/src/compiler/preprocessor/DiagnosticsBase.cpp
index fcb24e4..2b5e57c 100644
--- a/src/compiler/preprocessor/DiagnosticsBase.cpp
+++ b/src/compiler/preprocessor/DiagnosticsBase.cpp
@@ -82,6 +82,8 @@
return "Too many arguments for macro";
case PP_MACRO_DUPLICATE_PARAMETER_NAMES:
return "duplicate macro parameter name";
+ case PP_MACRO_INVOCATION_CHAIN_TOO_DEEP:
+ return "macro invocation chain too deep";
case PP_CONDITIONAL_ENDIF_WITHOUT_IF:
return "unexpected #endif found without a matching #if";
case PP_CONDITIONAL_ELSE_WITHOUT_IF:
diff --git a/src/compiler/preprocessor/DiagnosticsBase.h b/src/compiler/preprocessor/DiagnosticsBase.h
index 6c3fe9e..fec1ad7 100644
--- a/src/compiler/preprocessor/DiagnosticsBase.h
+++ b/src/compiler/preprocessor/DiagnosticsBase.h
@@ -48,6 +48,7 @@
PP_MACRO_TOO_FEW_ARGS,
PP_MACRO_TOO_MANY_ARGS,
PP_MACRO_DUPLICATE_PARAMETER_NAMES,
+ PP_MACRO_INVOCATION_CHAIN_TOO_DEEP,
PP_CONDITIONAL_ENDIF_WITHOUT_IF,
PP_CONDITIONAL_ELSE_WITHOUT_IF,
PP_CONDITIONAL_ELSE_AFTER_ELSE,
diff --git a/src/compiler/preprocessor/DirectiveParser.cpp b/src/compiler/preprocessor/DirectiveParser.cpp
index 643f73b..a268c80 100644
--- a/src/compiler/preprocessor/DirectiveParser.cpp
+++ b/src/compiler/preprocessor/DirectiveParser.cpp
@@ -202,14 +202,16 @@
DirectiveParser::DirectiveParser(Tokenizer *tokenizer,
MacroSet *macroSet,
Diagnostics *diagnostics,
- DirectiveHandler *directiveHandler)
+ DirectiveHandler *directiveHandler,
+ int maxMacroExpansionDepth)
: mPastFirstStatement(false),
mSeenNonPreprocessorToken(false),
mTokenizer(tokenizer),
mMacroSet(macroSet),
mDiagnostics(diagnostics),
mDirectiveHandler(directiveHandler),
- mShaderVersion(100)
+ mShaderVersion(100),
+ mMaxMacroExpansionDepth(maxMacroExpansionDepth)
{
}
@@ -843,7 +845,7 @@
bool parsedFileNumber = false;
int line = 0, file = 0;
- MacroExpander macroExpander(mTokenizer, mMacroSet, mDiagnostics);
+ MacroExpander macroExpander(mTokenizer, mMacroSet, mDiagnostics, mMaxMacroExpansionDepth);
// Lex the first token after "#line" so we can check it for EOD.
macroExpander.lex(token);
@@ -952,7 +954,7 @@
ASSERT((getDirective(token) == DIRECTIVE_IF) || (getDirective(token) == DIRECTIVE_ELIF));
DefinedParser definedParser(mTokenizer, mMacroSet, mDiagnostics);
- MacroExpander macroExpander(&definedParser, mMacroSet, mDiagnostics);
+ MacroExpander macroExpander(&definedParser, mMacroSet, mDiagnostics, mMaxMacroExpansionDepth);
ExpressionParser expressionParser(¯oExpander, mDiagnostics);
int expression = 0;
diff --git a/src/compiler/preprocessor/DirectiveParser.h b/src/compiler/preprocessor/DirectiveParser.h
index f0e889c..c14e9c0 100644
--- a/src/compiler/preprocessor/DirectiveParser.h
+++ b/src/compiler/preprocessor/DirectiveParser.h
@@ -24,7 +24,8 @@
DirectiveParser(Tokenizer *tokenizer,
MacroSet *macroSet,
Diagnostics *diagnostics,
- DirectiveHandler *directiveHandler);
+ DirectiveHandler *directiveHandler,
+ int maxMacroExpansionDepth);
void lex(Token *token) override;
@@ -76,6 +77,7 @@
Diagnostics *mDiagnostics;
DirectiveHandler *mDirectiveHandler;
int mShaderVersion;
+ int mMaxMacroExpansionDepth;
};
} // namespace pp
diff --git a/src/compiler/preprocessor/MacroExpander.cpp b/src/compiler/preprocessor/MacroExpander.cpp
index 11f677e..9605c64 100644
--- a/src/compiler/preprocessor/MacroExpander.cpp
+++ b/src/compiler/preprocessor/MacroExpander.cpp
@@ -77,11 +77,15 @@
mExpander->mMacrosToReenable.clear();
}
-MacroExpander::MacroExpander(Lexer *lexer, MacroSet *macroSet, Diagnostics *diagnostics)
+MacroExpander::MacroExpander(Lexer *lexer,
+ MacroSet *macroSet,
+ Diagnostics *diagnostics,
+ int allowedMacroExpansionDepth)
: mLexer(lexer),
mMacroSet(macroSet),
mDiagnostics(diagnostics),
mTotalTokensInContexts(0),
+ mAllowedMacroExpansionDepth(allowedMacroExpansionDepth),
mDeferReenablingMacros(false)
{
}
@@ -377,7 +381,13 @@
for (auto &arg : *args)
{
TokenLexer lexer(&arg);
- MacroExpander expander(&lexer, mMacroSet, mDiagnostics);
+ if (mAllowedMacroExpansionDepth < 1)
+ {
+ mDiagnostics->report(Diagnostics::PP_MACRO_INVOCATION_CHAIN_TOO_DEEP, token.location,
+ token.text);
+ return false;
+ }
+ MacroExpander expander(&lexer, mMacroSet, mDiagnostics, mAllowedMacroExpansionDepth - 1);
arg.clear();
expander.lex(&token);
diff --git a/src/compiler/preprocessor/MacroExpander.h b/src/compiler/preprocessor/MacroExpander.h
index 3a8450f..22a57c9 100644
--- a/src/compiler/preprocessor/MacroExpander.h
+++ b/src/compiler/preprocessor/MacroExpander.h
@@ -22,7 +22,10 @@
class MacroExpander : public Lexer
{
public:
- MacroExpander(Lexer *lexer, MacroSet *macroSet, Diagnostics *diagnostics);
+ MacroExpander(Lexer *lexer,
+ MacroSet *macroSet,
+ Diagnostics *diagnostics,
+ int allowedMacroExpansionDepth);
~MacroExpander() override;
void lex(Token *token) override;
@@ -68,6 +71,8 @@
std::vector<MacroContext *> mContextStack;
size_t mTotalTokensInContexts;
+ int mAllowedMacroExpansionDepth;
+
bool mDeferReenablingMacros;
std::vector<const Macro *> mMacrosToReenable;
diff --git a/src/compiler/preprocessor/Preprocessor.cpp b/src/compiler/preprocessor/Preprocessor.cpp
index 1709d66..5cebf4a 100644
--- a/src/compiler/preprocessor/Preprocessor.cpp
+++ b/src/compiler/preprocessor/Preprocessor.cpp
@@ -25,19 +25,26 @@
DirectiveParser directiveParser;
MacroExpander macroExpander;
- PreprocessorImpl(Diagnostics *diag, DirectiveHandler *directiveHandler)
+ PreprocessorImpl(Diagnostics *diag,
+ DirectiveHandler *directiveHandler,
+ const PreprocessorSettings &settings)
: diagnostics(diag),
tokenizer(diag),
- directiveParser(&tokenizer, ¯oSet, diag, directiveHandler),
- macroExpander(&directiveParser, ¯oSet, diag)
+ directiveParser(&tokenizer,
+ ¯oSet,
+ diag,
+ directiveHandler,
+ settings.maxMacroExpansionDepth),
+ macroExpander(&directiveParser, ¯oSet, diag, settings.maxMacroExpansionDepth)
{
}
};
Preprocessor::Preprocessor(Diagnostics *diagnostics,
- DirectiveHandler *directiveHandler)
+ DirectiveHandler *directiveHandler,
+ const PreprocessorSettings &settings)
{
- mImpl = new PreprocessorImpl(diagnostics, directiveHandler);
+ mImpl = new PreprocessorImpl(diagnostics, directiveHandler, settings);
}
Preprocessor::~Preprocessor()
diff --git a/src/compiler/preprocessor/Preprocessor.h b/src/compiler/preprocessor/Preprocessor.h
index cd69978..ecdbfff 100644
--- a/src/compiler/preprocessor/Preprocessor.h
+++ b/src/compiler/preprocessor/Preprocessor.h
@@ -19,10 +19,18 @@
struct PreprocessorImpl;
struct Token;
+struct PreprocessorSettings : angle::NonCopyable
+{
+ PreprocessorSettings() : maxMacroExpansionDepth(1000) {}
+ int maxMacroExpansionDepth;
+};
+
class Preprocessor : angle::NonCopyable
{
public:
- Preprocessor(Diagnostics *diagnostics, DirectiveHandler *directiveHandler);
+ Preprocessor(Diagnostics *diagnostics,
+ DirectiveHandler *directiveHandler,
+ const PreprocessorSettings &settings);
~Preprocessor();
// count: specifies the number of elements in the string and length arrays.
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 4ad597c..3066df4 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -99,7 +99,7 @@
mShaderVersion,
mShaderType,
resources.WEBGL_debug_shader_precision == 1),
- mPreprocessor(&mDiagnostics, &mDirectiveHandler),
+ mPreprocessor(&mDiagnostics, &mDirectiveHandler, pp::PreprocessorSettings()),
mScanner(nullptr),
mUsesFragData(false),
mUsesFragColor(false),