Skip to content

Commit

Permalink
[WebAssembly] Add validation to reloc section
Browse files Browse the repository at this point in the history
We now check relocations offsets are within range, and the relocation
index is valid.

Also updated tests which contained invalid Wasm files that were
previously not checked.

Differential Revision: https://reviews.llvm.org/D43684

llvm-svn: 326697
  • Loading branch information
NWilson committed Mar 5, 2018
1 parent f20222a commit b3748f7
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 6 deletions.
4 changes: 3 additions & 1 deletion llvm/include/llvm/Object/Wasm.h
Expand Up @@ -203,7 +203,9 @@ class WasmObjectFile : public ObjectFile {
bool isDefinedFunctionIndex(uint32_t Index) const;
bool isValidGlobalIndex(uint32_t Index) const;
bool isDefinedGlobalIndex(uint32_t Index) const;
bool isValidFunctionSymbolIndex(uint32_t Index) const;
bool isValidFunctionSymbol(uint32_t Index) const;
bool isValidGlobalSymbol(uint32_t Index) const;
bool isValidDataSymbol(uint32_t Index) const;
wasm::WasmFunction &getDefinedFunction(uint32_t Index);
wasm::WasmGlobal &getDefinedGlobal(uint32_t Index);

Expand Down
44 changes: 40 additions & 4 deletions llvm/lib/Object/WasmObjectFile.cpp
Expand Up @@ -339,7 +339,7 @@ Error WasmObjectFile::parseLinkingSection(const uint8_t *Ptr,
wasm::WasmInitFunc Init;
Init.Priority = readVaruint32(Ptr);
Init.Symbol = readVaruint32(Ptr);
if (!isValidFunctionSymbolIndex(Init.Symbol))
if (!isValidFunctionSymbol(Init.Symbol))
return make_error<GenericBinaryError>("Invalid function symbol: " +
Twine(Init.Symbol),
object_error::parse_failed);
Expand Down Expand Up @@ -554,29 +554,57 @@ Error WasmObjectFile::parseRelocSection(StringRef Name, const uint8_t *Ptr,
return make_error<GenericBinaryError>("Invalid section code",
object_error::parse_failed);
uint32_t RelocCount = readVaruint32(Ptr);
uint32_t LastOffset = 0;
uint32_t EndOffset = Section->Content.size();
while (RelocCount--) {
wasm::WasmRelocation Reloc;
memset(&Reloc, 0, sizeof(Reloc));
wasm::WasmRelocation Reloc = {};
Reloc.Type = readVaruint32(Ptr);
Reloc.Offset = readVaruint32(Ptr);
Reloc.Index = readVaruint32(Ptr);
switch (Reloc.Type) {
case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
if (!isValidFunctionSymbol(Reloc.Index))
return make_error<GenericBinaryError>("Bad relocation function index",
object_error::parse_failed);
break;
case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
if (Reloc.Index >= Signatures.size())
return make_error<GenericBinaryError>("Bad relocation type index",
object_error::parse_failed);
break;
case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
if (!isValidGlobalSymbol(Reloc.Index))
return make_error<GenericBinaryError>("Bad relocation global index",
object_error::parse_failed);
break;
case wasm::R_WEBASSEMBLY_MEMORY_ADDR_LEB:
case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32:
if (!isValidDataSymbol(Reloc.Index))
return make_error<GenericBinaryError>("Bad relocation data index",
object_error::parse_failed);
Reloc.Addend = readVarint32(Ptr);
break;
default:
return make_error<GenericBinaryError>("Bad relocation type: " +
Twine(Reloc.Type),
object_error::parse_failed);
}

// Relocations must fit inside the section, and must appear in order. They
// also shouldn't overlap a function/element boundary, but we don't bother
// to check that.
uint64_t Size = 5;
if (Reloc.Type == wasm::R_WEBASSEMBLY_TABLE_INDEX_I32 ||
Reloc.Type == wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32)
Size = 4;
if (Reloc.Offset < LastOffset || Reloc.Offset + Size > EndOffset)
return make_error<GenericBinaryError>("Bad relocation offset",
object_error::parse_failed);
LastOffset = Reloc.Offset;

Section->Relocations.push_back(Reloc);
}
if (Ptr != End)
Expand Down Expand Up @@ -787,10 +815,18 @@ bool WasmObjectFile::isDefinedGlobalIndex(uint32_t Index) const {
return Index >= NumImportedGlobals && isValidGlobalIndex(Index);
}

bool WasmObjectFile::isValidFunctionSymbolIndex(uint32_t Index) const {
bool WasmObjectFile::isValidFunctionSymbol(uint32_t Index) const {
return Index < Symbols.size() && Symbols[Index].isTypeFunction();
}

bool WasmObjectFile::isValidGlobalSymbol(uint32_t Index) const {
return Index < Symbols.size() && Symbols[Index].isTypeGlobal();
}

bool WasmObjectFile::isValidDataSymbol(uint32_t Index) const {
return Index < Symbols.size() && Symbols[Index].isTypeData();
}

wasm::WasmFunction &WasmObjectFile::getDefinedFunction(uint32_t Index) {
assert(isDefinedFunctionIndex(Index));
return Functions[Index - NumImportedFunctions];
Expand Down
13 changes: 13 additions & 0 deletions llvm/test/ObjectYAML/wasm/code_section.yaml
Expand Up @@ -37,6 +37,19 @@ Sections:
- Type: I32
Count: 1
Body: 108180808000210020000F0B
- Type: CUSTOM
Name: linking
SymbolTable:
- Index: 0
Kind: FUNCTION
Name: func1
Flags: [ ]
Function: 0
- Index: 1
Kind: FUNCTION
Name: func2
Flags: [ ]
Function: 1
...
# CHECK: --- !WASM
# CHECK: FileHeader:
Expand Down
12 changes: 11 additions & 1 deletion llvm/test/ObjectYAML/wasm/data_section.yaml
Expand Up @@ -22,6 +22,16 @@ Sections:
Index: 0
Offset: 0x00000006
Addend: -6
- Type: CUSTOM
Name: linking
SymbolTable:
- Index: 0
Kind: DATA
Name: dataSymbol
Flags: [ ]
Segment: 0
Offset: 0
Size: 4
...
# CHECK: --- !WASM
# CHECK: FileHeader:
Expand All @@ -44,4 +54,4 @@ Sections:
# CHECK-NEXT: Opcode: I32_CONST
# CHECK-NEXT: Value: 4
# CHECK-NEXT: Content: '10001000'
# CHECK-NEXT: ...
# CHECK-NEXT: - Type: CUSTOM

0 comments on commit b3748f7

Please sign in to comment.