Skip to content

Commit

Permalink
Change SkipRestOfBlock to be iterative instead of recursive to avoid …
Browse files Browse the repository at this point in the history
…stack

overflows on invalid input with many unbalanced `{` characters.

PiperOrigin-RevId: 561412549
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Sep 5, 2023
1 parent f7d6dd1 commit 0978165
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/google/protobuf/compiler/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

#include <float.h>

#include <cstddef>
#include <cstdint>
#include <limits>
#include <string>
Expand Down Expand Up @@ -555,14 +556,15 @@ void Parser::SkipStatement() {
}

void Parser::SkipRestOfBlock() {
size_t block_count = 1;
while (true) {
if (AtEnd()) {
return;
} else if (LookingAtType(io::Tokenizer::TYPE_SYMBOL)) {
if (TryConsumeEndOfDeclaration("}", nullptr)) {
return;
if (--block_count == 0) break;
} else if (TryConsume("{")) {
SkipRestOfBlock();
++block_count;
}
}
input_->Next();
Expand Down
18 changes: 15 additions & 3 deletions src/google/protobuf/compiler/parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ class ParserTest : public testing::Test {
ParserTest() : require_syntax_identifier_(false) {}

// Set up the parser to parse the given text.
void SetupParser(const char* text) {
raw_input_ = absl::make_unique<io::ArrayInputStream>(text, strlen(text));
void SetupParser(absl::string_view text) {
raw_input_ =
absl::make_unique<io::ArrayInputStream>(text.data(), text.size());
input_ =
absl::make_unique<io::Tokenizer>(raw_input_.get(), &error_collector_);
parser_ = absl::make_unique<Parser>();
Expand Down Expand Up @@ -169,7 +170,8 @@ class ParserTest : public testing::Test {

// Same as above but does not expect that the parser parses the complete
// input.
void ExpectHasEarlyExitErrors(const char* text, const char* expected_errors) {
void ExpectHasEarlyExitErrors(absl::string_view text,
absl::string_view expected_errors) {
SetupParser(text);
SourceLocationTable source_locations;
parser_->RecordSourceLocationsTo(&source_locations);
Expand Down Expand Up @@ -286,6 +288,16 @@ TEST_F(ParserTest, WarnIfFieldNameContainsNumberImmediatelyFollowUnderscore) {
"song_name_1.") != std::string::npos);
}

TEST_F(ParserTest, RegressionNestedOpenBraceDoNotStackOverflow) {
std::string input("edition=\"a\000;", 12);
input += std::string(100000, '{');
ExpectHasEarlyExitErrors(
input,
"0:10: Unexpected end of string.\n"
"0:10: Invalid control characters encountered in text.\n"
"0:12: Expected top-level statement (e.g. \"message\").\n");
}

// ===================================================================

typedef ParserTest ParseMessageTest;
Expand Down

0 comments on commit 0978165

Please sign in to comment.