Fix function parsing for clang debug builds of wrap files#6
Open
kieranclancy wants to merge 5 commits into
Open
Fix function parsing for clang debug builds of wrap files#6kieranclancy wants to merge 5 commits into
kieranclancy wants to merge 5 commits into
Conversation
memcpy() is not safe to use on overlapping regions.
A number of string [r]find() operations were not being checked for return values (especially == npos) and the resulting indices could have resulted in buffer overflow or other errors. This became apparent when trying to debug a crash when running bind_fakes on an object file produced by clang++ with debug symbols enabled. Adds checks for these return values and either returns an empty result or at least a runtime exception where appropriate rather than a buffer overflow.
The ReadFunctionsList did not check that the string between the start and end markers was a complete string containing no NUL characters. This meant, in particular with at least one version of clang, that fragments of the start and end markers in the object file could be misinterpreted and parsed as a prototype string, leading to a buffer overflow due to subsequent mishandling of the malformed data between the two markers. The fix here adds a function to check that the start and end markers do not contain a NUL character between them. --- BEGIN example wrap.cpp --- extern "C" void f(void); #include "powerfake.h" WRAP_FUNCTION(f); --- END example wrap.cpp --- When compiled with `clang++ -c -g -o wrap.o wrap.cpp` the resulting object file causes crashes in bind_fakes due to start/end fragments being emitted in debug symbols. (Tested with clang version 19.1.7)
Adds tests to exercise a number of cases where find() results on strings were not previously being checked for invalid values and this could lead to buffer overflows.
These tests include a test of a specific blob produced by clang++ that had a fragment of the prototype start string and the prototype end string with some malformed data between them.
Owner
|
Thank you for this PR and sorry for the late reply. I'll hopefully look into it in a few days. :) And happy to see that you found this project useful! 😊 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
First of all many thanks for your project. I've wanted something like this tool for a while and so was happy to stumble upon it.
Here's the issue I ran into:
When I use
clang++to build a wrap file, even a simple one such as:And compile with
clang++ -c -g -o wrap.o wrap.cpp(clang 19.1.17 on Linux), the resulting object file contains binary data such as::Note that the correct prototype string starts on
00001240, but there are fragments of the start marker at00001185and end marker at000011fdthat are misinterpreted bybind_fakesas a prototype string. This is because the current parsing functions do not check for nul characters between the markers, and as a result it passes the malformed data to various string parsing functions that in turn cause an (attempted) buffer overflow:$ bind_fakes --standalone --output-prefix mylib.powerfake --symbol-files mylib.a --wrapper-files wrap.o Exception: basic_string_view::substr: __pos (which is 18446744073709551615) > __size (which is 6)This PR fixes the root cause in
ReadFunctionsListwhere it does not check for nul characters between the start and end markers, and also adds some minimal return value checks to the other parsing functions so that they don't crash on bad data.Unit tests are added to exercise these fixed code paths.
While I was there I also noticed a
memcpy()of data from the same buffer into itself, which is undefined behaviour, so have changed this to amemmove().I am happy to release these patches under the boost software license 1.0, if you should want to merge this into the project. I've tried to keep my coding style consistent with the surrounding code.