From e7257eb2fbd560305ffb672e7e9bafb8848191cc Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 21 Jun 2021 19:36:26 +0000 Subject: [PATCH] reader/wgsl: Prevent stack overflow in unary_expression() Fixed: chromium:1209237 Change-Id: I14b4777aaee3ee7a34baf8a218db28f54b81af84 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55253 Kokoro: Kokoro Reviewed-by: James Price Commit-Queue: Ben Clayton --- src/reader/wgsl/parser_impl.cc | 25 +++++++++++++------ src/reader/wgsl/parser_impl.h | 2 +- .../wgsl/parser_impl_const_expr_test.cc | 14 +++++++++++ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index bb6c886f10..71ba2d04ea 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -52,10 +52,10 @@ using Expect = ParserImpl::Expect; template using Maybe = ParserImpl::Maybe; -/// Controls the maximum number of times we'll call into the sync() function -/// from itself. This is to guard against stack overflow when there is an -/// excessive number of blocks. -constexpr uint32_t kMaxSyncDepth = 128; +/// Controls the maximum number of times we'll call into the sync() and +/// unary_expression() functions from themselves. This is to guard against stack +/// overflow when there is an excessive number of blocks. +constexpr uint32_t kMaxParseDepth = 128; /// The maximum number of tokens to look ahead to try and sync the /// parser on error. @@ -2327,7 +2327,18 @@ Maybe ParserImpl::unary_expression() { return singular_expression(); } + if (parse_depth_ >= kMaxParseDepth) { + // We've hit a maximum parser recursive depth. + // We can't call into unary_expression() as we might stack overflow. + // Instead, report an error + add_error(peek(), "maximum parser recursive depth reached"); + return Failure::kErrored; + } + + ++parse_depth_; auto expr = unary_expression(); + --parse_depth_; + if (expr.errored) { return Failure::kErrored; } @@ -3268,7 +3279,7 @@ T ParserImpl::expect_lt_gt_block(const std::string& use, F&& body) { template T ParserImpl::sync(Token::Type tok, F&& body) { - if (sync_depth_ >= kMaxSyncDepth) { + if (parse_depth_ >= kMaxParseDepth) { // We've hit a maximum parser recursive depth. // We can't call into body() as we might stack overflow. // Instead, report an error... @@ -3282,9 +3293,9 @@ T ParserImpl::sync(Token::Type tok, F&& body) { sync_tokens_.push_back(tok); - ++sync_depth_; + ++parse_depth_; auto result = body(); - --sync_depth_; + --parse_depth_; if (sync_tokens_.back() != tok) { TINT_ICE(builder_.Diagnostics()) << "sync_tokens is out of sync"; diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index 0fa277350f..55cd25a753 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -871,7 +871,7 @@ class ParserImpl { std::deque token_queue_; Token last_token_; bool synchronized_ = true; - uint32_t sync_depth_ = 0; + uint32_t parse_depth_ = 0; std::vector sync_tokens_; int silence_errors_ = 0; std::unordered_map registered_types_; diff --git a/src/reader/wgsl/parser_impl_const_expr_test.cc b/src/reader/wgsl/parser_impl_const_expr_test.cc index 5a43c48341..db05cf91f8 100644 --- a/src/reader/wgsl/parser_impl_const_expr_test.cc +++ b/src/reader/wgsl/parser_impl_const_expr_test.cc @@ -154,6 +154,20 @@ TEST_F(ParserImplTest, ConstExpr_Recursion) { EXPECT_EQ(p->error(), "1:517: maximum parser recursive depth reached"); } +TEST_F(ParserImplTest, UnaryOp_Recursion) { + std::stringstream out; + for (size_t i = 0; i < 200; i++) { + out << "!"; + } + out << "1.0"; + auto p = parser(out.str()); + auto e = p->unary_expression(); + ASSERT_TRUE(p->has_error()); + ASSERT_TRUE(e.errored); + ASSERT_EQ(e.value, nullptr); + EXPECT_EQ(p->error(), "1:130: maximum parser recursive depth reached"); +} + } // namespace } // namespace wgsl } // namespace reader