[lld-macho] Avoid infinite recursion when parsing corrupted export tries #152569
+29
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If an export trie is encoded incorrectly, and one of the children offsets points back to one of the nodes earlier in the serialization, the current code will end up in an infinite recursion, and eventually fail exhausting the available memory. The failure can be avoided if, before recursing, one checks that the offset is valid, that is, that the offset is beyond the current position. This is similar to a check done by llvm-objdump which reports the trie being corrupted.
|
@llvm/pr-subscribers-lld Author: Daniel Rodríguez Troitiño (drodriguez) ChangesIf an export trie is encoded incorrectly, and one of the children offsets points back to one of the nodes earlier in the serialization, the current code will end up in an infinite recursion, and eventually fail exhausting the available memory. The failure can be avoided if, before recursing, one checks that the offset is valid, that is, that the offset is beyond the current position. This is similar to a check done by llvm-objdump which reports the trie being corrupted. Full diff: https://github.com/llvm/llvm-project/pull/152569.diff 5 Files Affected:
diff --git a/lld/MachO/ExportTrie.cpp b/lld/MachO/ExportTrie.cpp
index 303eda416c235..8ba48ebc14a72 100644
--- a/lld/MachO/ExportTrie.cpp
+++ b/lld/MachO/ExportTrie.cpp
@@ -296,13 +296,14 @@ namespace {
// Parse a serialized trie and invoke a callback for each entry.
class TrieParser {
public:
- TrieParser(const uint8_t *buf, size_t size, const TrieEntryCallback &callback)
- : start(buf), end(start + size), callback(callback) {}
+ TrieParser(const std::string &fileName, const uint8_t *buf, size_t size, const TrieEntryCallback &callback)
+ : fileName(fileName), start(buf), end(start + size), callback(callback) {}
void parse(const uint8_t *buf, const Twine &cumulativeString);
void parse() { parse(start, ""); }
+ const std::string fileName;
const uint8_t *start;
const uint8_t *end;
const TrieEntryCallback &callback;
@@ -312,7 +313,7 @@ class TrieParser {
void TrieParser::parse(const uint8_t *buf, const Twine &cumulativeString) {
if (buf >= end)
- fatal("Node offset points outside export section");
+ fatal(fileName + ": export trie node offset points outside export section");
unsigned ulebSize;
uint64_t terminalSize = decodeULEB128(buf, &ulebSize);
@@ -331,14 +332,17 @@ void TrieParser::parse(const uint8_t *buf, const Twine &cumulativeString) {
buf += substring.size() + 1;
offset = decodeULEB128(buf, &ulebSize);
buf += ulebSize;
+ if (start + offset < buf)
+ fatal(fileName + ": export trie child node offset points before parent node");
parse(start + offset, cumulativeString + substring);
}
}
-void macho::parseTrie(const uint8_t *buf, size_t size,
+void macho::parseTrie(const std::string &fileName,
+ const uint8_t *buf, size_t size,
const TrieEntryCallback &callback) {
if (size == 0)
return;
- TrieParser(buf, size, callback).parse();
+ TrieParser(fileName, buf, size, callback).parse();
}
diff --git a/lld/MachO/ExportTrie.h b/lld/MachO/ExportTrie.h
index aa7e3b0d4a14b..5a5565d52bcc8 100644
--- a/lld/MachO/ExportTrie.h
+++ b/lld/MachO/ExportTrie.h
@@ -41,7 +41,7 @@ class TrieBuilder {
using TrieEntryCallback =
llvm::function_ref<void(const llvm::Twine & /*name*/, uint64_t /*flags*/)>;
-void parseTrie(const uint8_t *buf, size_t size, const TrieEntryCallback &);
+void parseTrie(const std::string &fileName, const uint8_t *buf, size_t size, const TrieEntryCallback &);
} // namespace lld::macho
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 3b3023a94166f..607867325be46 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1789,7 +1789,7 @@ void DylibFile::parseExportedSymbols(uint32_t offset, uint32_t size) {
auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
std::vector<TrieEntry> entries;
// Find all the $ld$* symbols to process first.
- parseTrie(buf + offset, size, [&](const Twine &name, uint64_t flags) {
+ parseTrie(toString(this), buf + offset, size, [&](const Twine &name, uint64_t flags) {
StringRef savedName = saver().save(name);
if (handleLDSymbol(savedName))
return;
diff --git a/lld/test/MachO/invalid/Inputs/macho-trie-node-loop b/lld/test/MachO/invalid/Inputs/macho-trie-node-loop
new file mode 100755
index 0000000000000..b94dfa2610e9f
Binary files /dev/null and b/lld/test/MachO/invalid/Inputs/macho-trie-node-loop differ
diff --git a/lld/test/MachO/invalid/export-trie-node-loop.s b/lld/test/MachO/invalid/export-trie-node-loop.s
new file mode 100644
index 0000000000000..d871709971dd4
--- /dev/null
+++ b/lld/test/MachO/invalid/export-trie-node-loop.s
@@ -0,0 +1,9 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: not %lld -o %t %t.o %S/Inputs/macho-trie-node-loop 2>&1 | FileCheck %s
+# CHECK: error:
+# CHECK-SAME: /Inputs/macho-trie-node-loop: export trie child node offset points before parent node
+
+.globl _main
+_main:
+ ret
|
|
@llvm/pr-subscribers-lld-macho Author: Daniel Rodríguez Troitiño (drodriguez) ChangesIf an export trie is encoded incorrectly, and one of the children offsets points back to one of the nodes earlier in the serialization, the current code will end up in an infinite recursion, and eventually fail exhausting the available memory. The failure can be avoided if, before recursing, one checks that the offset is valid, that is, that the offset is beyond the current position. This is similar to a check done by llvm-objdump which reports the trie being corrupted. Full diff: https://github.com/llvm/llvm-project/pull/152569.diff 5 Files Affected:
diff --git a/lld/MachO/ExportTrie.cpp b/lld/MachO/ExportTrie.cpp
index 303eda416c235..8ba48ebc14a72 100644
--- a/lld/MachO/ExportTrie.cpp
+++ b/lld/MachO/ExportTrie.cpp
@@ -296,13 +296,14 @@ namespace {
// Parse a serialized trie and invoke a callback for each entry.
class TrieParser {
public:
- TrieParser(const uint8_t *buf, size_t size, const TrieEntryCallback &callback)
- : start(buf), end(start + size), callback(callback) {}
+ TrieParser(const std::string &fileName, const uint8_t *buf, size_t size, const TrieEntryCallback &callback)
+ : fileName(fileName), start(buf), end(start + size), callback(callback) {}
void parse(const uint8_t *buf, const Twine &cumulativeString);
void parse() { parse(start, ""); }
+ const std::string fileName;
const uint8_t *start;
const uint8_t *end;
const TrieEntryCallback &callback;
@@ -312,7 +313,7 @@ class TrieParser {
void TrieParser::parse(const uint8_t *buf, const Twine &cumulativeString) {
if (buf >= end)
- fatal("Node offset points outside export section");
+ fatal(fileName + ": export trie node offset points outside export section");
unsigned ulebSize;
uint64_t terminalSize = decodeULEB128(buf, &ulebSize);
@@ -331,14 +332,17 @@ void TrieParser::parse(const uint8_t *buf, const Twine &cumulativeString) {
buf += substring.size() + 1;
offset = decodeULEB128(buf, &ulebSize);
buf += ulebSize;
+ if (start + offset < buf)
+ fatal(fileName + ": export trie child node offset points before parent node");
parse(start + offset, cumulativeString + substring);
}
}
-void macho::parseTrie(const uint8_t *buf, size_t size,
+void macho::parseTrie(const std::string &fileName,
+ const uint8_t *buf, size_t size,
const TrieEntryCallback &callback) {
if (size == 0)
return;
- TrieParser(buf, size, callback).parse();
+ TrieParser(fileName, buf, size, callback).parse();
}
diff --git a/lld/MachO/ExportTrie.h b/lld/MachO/ExportTrie.h
index aa7e3b0d4a14b..5a5565d52bcc8 100644
--- a/lld/MachO/ExportTrie.h
+++ b/lld/MachO/ExportTrie.h
@@ -41,7 +41,7 @@ class TrieBuilder {
using TrieEntryCallback =
llvm::function_ref<void(const llvm::Twine & /*name*/, uint64_t /*flags*/)>;
-void parseTrie(const uint8_t *buf, size_t size, const TrieEntryCallback &);
+void parseTrie(const std::string &fileName, const uint8_t *buf, size_t size, const TrieEntryCallback &);
} // namespace lld::macho
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 3b3023a94166f..607867325be46 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1789,7 +1789,7 @@ void DylibFile::parseExportedSymbols(uint32_t offset, uint32_t size) {
auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
std::vector<TrieEntry> entries;
// Find all the $ld$* symbols to process first.
- parseTrie(buf + offset, size, [&](const Twine &name, uint64_t flags) {
+ parseTrie(toString(this), buf + offset, size, [&](const Twine &name, uint64_t flags) {
StringRef savedName = saver().save(name);
if (handleLDSymbol(savedName))
return;
diff --git a/lld/test/MachO/invalid/Inputs/macho-trie-node-loop b/lld/test/MachO/invalid/Inputs/macho-trie-node-loop
new file mode 100755
index 0000000000000..b94dfa2610e9f
Binary files /dev/null and b/lld/test/MachO/invalid/Inputs/macho-trie-node-loop differ
diff --git a/lld/test/MachO/invalid/export-trie-node-loop.s b/lld/test/MachO/invalid/export-trie-node-loop.s
new file mode 100644
index 0000000000000..d871709971dd4
--- /dev/null
+++ b/lld/test/MachO/invalid/export-trie-node-loop.s
@@ -0,0 +1,9 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: not %lld -o %t %t.o %S/Inputs/macho-trie-node-loop 2>&1 | FileCheck %s
+# CHECK: error:
+# CHECK-SAME: /Inputs/macho-trie-node-loop: export trie child node offset points before parent node
+
+.globl _main
+_main:
+ ret
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an export trie is encoded incorrectly, and one of the children offsets points back to one of the nodes earlier in the serialization, the current code will end up in an infinite recursion, and eventually fail exhausting the available memory.
The failure can be avoided if, before recursing, one checks that the offset is valid, that is, that the offset is beyond the current position. This is similar to a check done by llvm-objdump which reports the trie being corrupted.