Skip to content

Add declaration limit checks to parser #2736

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

Merged
zherczeg merged 1 commit into from
Apr 2, 2026
+35 −1
Merged

Add declaration limit checks to parser #2736

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
3 changes: 3 additions & 0 deletions include/wabt/wast-parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ class WastParser {
// synchronized.
Result Synchronize(SynchronizeFunc);

// Check the maximum allowed declarations.
Result CheckIndexRange(Location& loc, size_t size, const char* decl);

Result ParseVarText(Token& token, std::string* out_text);
Result ParseBindVarOpt(std::string* name);
Result ParseVar(Var* out_var);
Expand Down
33 changes: 32 additions & 1 deletion src/wast-parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,16 @@ Result WastParser::Synchronize(SynchronizeFunc func) {
return Result::Error;
}

Result WastParser::CheckIndexRange(Location& loc,
size_t size,
const char* decl) {
if (size >= kInvalidIndex) {
Copy link
Copy Markdown
Member

@sbc100 sbc100 Apr 2, 2026

Choose a reason for hiding this comment

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

I guess there are likely implemenation limits that are much lower than 2^32 that we could apply here?

With this check we are going to get OOM way before we hit it on 32-bit platforms right?

Copy link
Copy Markdown
Collaborator Author

@zherczeg zherczeg Apr 2, 2026

Choose a reason for hiding this comment

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

Yes, on 32 bit platforms this cannot happen. What do you suggest? A 64 bit specific macro? Or this patch is a bad idea?

Copy link
Copy Markdown
Member

@sbc100 sbc100 Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think this patch is good, but we we might also want to enforce much lower limits such as the ones defined in https://webassembly.github.io/spec/core/appendix/implementation.html#implementation-limitations.

That doesn't have to happen as part of this PR though.

Error(loc, "too many %s declarations", decl);
return Result::Error;
}
return Result::Ok;
}

void WastParser::ErrorUnlessOpcodeEnabled(const Token& token) {
Opcode opcode = token.opcode();
if (!opcode.IsEnabled(options_->features)) {
Expand Down Expand Up @@ -1506,6 +1516,7 @@ Result WastParser::ParseDataModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Data);
CHECK_RESULT(CheckIndexRange(loc, module->data_segments.size(), "data"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));
auto field = std::make_unique<DataSegmentModuleField>(loc, name);
Expand Down Expand Up @@ -1543,6 +1554,7 @@ Result WastParser::ParseElemModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Elem);
CHECK_RESULT(CheckIndexRange(loc, module->elem_segments.size(), "elem"));

// With MVP text format the name here was intended to refer to the table
// that the elem segment was part of, but we never did anything with this name
Expand Down Expand Up @@ -1620,7 +1632,7 @@ Result WastParser::ParseTagModuleField(Module* module) {
EXPECT(Lpar);
EXPECT(Tag);
Location loc = GetLocation();
Copy link
Copy Markdown
Collaborator Author

@zherczeg zherczeg Apr 2, 2026

Choose a reason for hiding this comment

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

Usually the location refers to the field type token, not the token after it. Perhaps this is a mistake.


CHECK_RESULT(CheckIndexRange(loc, module->tags.size(), "tag"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));

Expand All @@ -1629,6 +1641,7 @@ Result WastParser::ParseTagModuleField(Module* module) {

if (PeekMatchLpar(TokenType::Import)) {
CheckImportOrdering(module);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
auto import = std::make_unique<TagImport>(name);
Tag& tag = import->tag;
CHECK_RESULT(ParseInlineImport(import.get()));
Expand All @@ -1655,7 +1668,9 @@ Result WastParser::ParseExportModuleField(Module* module) {
WABT_TRACE(ParseExportModuleField);
EXPECT(Lpar);
auto field = std::make_unique<ExportModuleField>(GetLocation());
Location loc = GetLocation();
EXPECT(Export);
CHECK_RESULT(CheckIndexRange(loc, module->exports.size(), "export"));
CHECK_RESULT(ParseQuotedText(&field->export_.name));
CHECK_RESULT(ParseExportDesc(&field->export_));
EXPECT(Rpar);
Expand All @@ -1668,6 +1683,7 @@ Result WastParser::ParseFuncModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Func);
CHECK_RESULT(CheckIndexRange(loc, module->funcs.size(), "func"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));

Expand All @@ -1676,6 +1692,7 @@ Result WastParser::ParseFuncModuleField(Module* module) {

if (PeekMatchLpar(TokenType::Import)) {
CheckImportOrdering(module);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
auto import = std::make_unique<FuncImport>(name);
Func& func = import->func;
CHECK_RESULT(ParseInlineImport(import.get()));
Expand Down Expand Up @@ -1725,6 +1742,7 @@ Result WastParser::ParseTypeModuleField(Module* module) {
CHECK_RESULT(ParseBindVarOpt(&name));
EXPECT(Lpar);
Location loc = GetLocation();
CHECK_RESULT(CheckIndexRange(loc, module->types.size(), "type"));

if (Match(TokenType::Func)) {
auto func_type = std::make_unique<FuncType>(name);
Expand Down Expand Up @@ -1802,6 +1820,7 @@ Result WastParser::ParseGlobalModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Global);
CHECK_RESULT(CheckIndexRange(loc, module->globals.size(), "global"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));

Expand All @@ -1810,6 +1829,7 @@ Result WastParser::ParseGlobalModuleField(Module* module) {

if (PeekMatchLpar(TokenType::Import)) {
CheckImportOrdering(module);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
auto import = std::make_unique<GlobalImport>(name);
CHECK_RESULT(ParseInlineImport(import.get()));
CHECK_RESULT(ParseGlobalType(&import->global));
Expand All @@ -1835,6 +1855,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
Location loc = GetLocation();
CheckImportOrdering(module);
EXPECT(Import);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
std::string module_name;
std::string field_name;
CHECK_RESULT(ParseQuotedText(&module_name));
Expand All @@ -1846,6 +1867,7 @@ Result WastParser::ParseImportModuleField(Module* module) {

switch (Peek()) {
case TokenType::Func: {
CHECK_RESULT(CheckIndexRange(loc, module->funcs.size(), "func"));
DropToken();
CHECK_RESULT(ParseBindVarOpt(&name));
auto import = std::make_unique<FuncImport>(name);
Expand All @@ -1859,6 +1881,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
}

case TokenType::Table: {
CHECK_RESULT(CheckIndexRange(loc, module->tables.size(), "table"));
DropToken();
CHECK_RESULT(ParseBindVarOpt(&name));
auto import = std::make_unique<TableImport>(name);
Expand All @@ -1873,6 +1896,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
}

case TokenType::Memory: {
CHECK_RESULT(CheckIndexRange(loc, module->memories.size(), "memory"));
DropToken();
CHECK_RESULT(ParseBindVarOpt(&name));
auto import = std::make_unique<MemoryImport>(name);
Expand All @@ -1886,6 +1910,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
}

case TokenType::Global: {
CHECK_RESULT(CheckIndexRange(loc, module->globals.size(), "global"));
DropToken();
CHECK_RESULT(ParseBindVarOpt(&name));
auto import = std::make_unique<GlobalImport>(name);
Expand All @@ -1896,6 +1921,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
}

case TokenType::Tag: {
CHECK_RESULT(CheckIndexRange(loc, module->tags.size(), "tag"));
DropToken();
CHECK_RESULT(ParseBindVarOpt(&name));
auto import = std::make_unique<TagImport>(name);
Expand Down Expand Up @@ -1923,6 +1949,7 @@ Result WastParser::ParseMemoryModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Memory);
CHECK_RESULT(CheckIndexRange(loc, module->memories.size(), "memory"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));

Expand All @@ -1931,6 +1958,7 @@ Result WastParser::ParseMemoryModuleField(Module* module) {

if (PeekMatchLpar(TokenType::Import)) {
CheckImportOrdering(module);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
auto import = std::make_unique<MemoryImport>(name);
import->memory.page_size = WABT_DEFAULT_PAGE_SIZE;
CHECK_RESULT(ParseInlineImport(import.get()));
Expand Down Expand Up @@ -2004,6 +2032,7 @@ Result WastParser::ParseTableModuleField(Module* module) {
EXPECT(Lpar);
Location loc = GetLocation();
EXPECT(Table);
CHECK_RESULT(CheckIndexRange(loc, module->tables.size(), "table"));
std::string name;
CHECK_RESULT(ParseBindVarOpt(&name));

Expand All @@ -2012,6 +2041,7 @@ Result WastParser::ParseTableModuleField(Module* module) {

if (PeekMatchLpar(TokenType::Import)) {
CheckImportOrdering(module);
CHECK_RESULT(CheckIndexRange(loc, module->imports.size(), "import"));
auto import = std::make_unique<TableImport>(name);
CHECK_RESULT(ParseInlineImport(import.get()));
CHECK_RESULT(ParseLimitsIndex(&import->table.elem_limits));
Expand All @@ -2032,6 +2062,7 @@ Result WastParser::ParseTableModuleField(Module* module) {

EXPECT(Lpar);
EXPECT(Elem);
CHECK_RESULT(CheckIndexRange(loc, module->elem_segments.size(), "elem"));

auto elem_segment_field = std::make_unique<ElemSegmentModuleField>(loc);
ElemSegment& elem_segment = elem_segment_field->elem_segment;
Expand Down
Loading