[lldb][Linux] Fix potential out of bounds read of pr_fname #159375
Conversation
llvm#157170 added code that assigned pr_fname to another std::string member. In the lines before, we copy pr_fname using assign with a max length set to either the length of the string in pr_fname, or the size of pr_fname. Which is 16 bytes. struct ELFLinuxPrPsInfo { <...> char pr_fname[16]; The content of pr_fname can fill all 16 bytes, that's why we need the limit. This was not done for m_executable_name where it ended up calling the assignment from char* operator which could read on into the rest of the corefile in some cases. Likely wouldn't crash for reading out of bounds, but you would at least see some strange things in LLDB. Fix this by copying the std::string we already made for thread_data.name.
DavidSpickett
requested review from
dmpots and
GeorgeHuyubo
and removed request for
JDevlieghere and
dmpots
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) Changes#157170 added code that assigned pr_fname to another std::string member. In the lines before, we copy pr_fname using assign with a max length set to either the length of the string in pr_fname, or the size of pr_fname. Which is 16 bytes. struct ELFLinuxPrPsInfo { The content of pr_fname can fill all 16 bytes, that's why we need the limit. This was not done for m_executable_name where it ended up calling the assignment from char* operator which could read on into the rest of the corefile in some cases. Likely wouldn't crash for reading out of bounds, but you would at least see some strange things in LLDB. Fix this by copying the std::string we already made for thread_data.name. Full diff: https://github.com/llvm/llvm-project/pull/159375.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 8f5f1242116f5..38bf13543c617 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -952,7 +952,7 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) {
return status.ToError();
thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)));
SetID(prpsinfo.pr_pid);
- m_executable_name = prpsinfo.pr_fname;
+ m_executable_name = thread_data.name;
break;
}
case ELF::NT_SIGINFO: {
|
|
This is unrelated to that since this fixes a Linux specific code path. The failing tests are all NetBSD cores and I confirmed we're not calling the Linux code accidentally there. |
|
To prevent any confusion working on fixing that issue, I will leave this PR open until that issue is resolved. |
|
#159588 already fixed this. |
#157170 added code that assigned pr_fname to another std::string member.
In the lines before, we copy pr_fname using assign with a max length set to either the length of the string in pr_fname, or the size of pr_fname. Which is 16 bytes.
struct ELFLinuxPrPsInfo {
<...>
char pr_fname[16];
The content of pr_fname can fill all 16 bytes, that's why we need the limit.
This was not done for m_executable_name where it ended up calling the assignment from char* operator which could read on into the rest of the corefile in some cases.
Likely wouldn't crash for reading out of bounds, but you would at least see some strange things in LLDB.
Fix this by copying the std::string we already made for thread_data.name.