[lldb] Fix out of bounds read in DataExtractor::GetCStr and add unit test that function.
Summary: The `if (*cstr_end == '\0')` in the previous code checked if the previous loop terminated because it found a null terminator or because it reached the end of the data. However, in the case that we hit the end of the data before finding a null terminator, `cstr_end` points behind the last byte in our data and `*cstr_end` reads the memory behind the array (which may be uninitialised) This patch just rewrites that function use `std::find` and adds the relevant unit tests. Reviewers: labath Reviewed By: labath Subscribers: abidh, JDevlieghere, lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D68773 llvm-svn: 374311
This commit is contained in:
parent
eb8b6fe745
commit
067bb1f546
|
@ -816,26 +816,25 @@ DataExtractor::CopyByteOrderedData(offset_t src_offset, offset_t src_len,
|
|||
// non-zero and there aren't enough available bytes, nullptr will be returned
|
||||
// and "offset_ptr" will not be updated.
|
||||
const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
|
||||
const char *cstr = reinterpret_cast<const char *>(PeekData(*offset_ptr, 1));
|
||||
if (cstr) {
|
||||
const char *cstr_end = cstr;
|
||||
const char *end = reinterpret_cast<const char *>(m_end);
|
||||
while (cstr_end < end && *cstr_end)
|
||||
++cstr_end;
|
||||
const char *start = reinterpret_cast<const char *>(PeekData(*offset_ptr, 1));
|
||||
// Already at the end of the data.
|
||||
if (!start)
|
||||
return nullptr;
|
||||
|
||||
// Now we are either at the end of the data or we point to the
|
||||
// NULL C string terminator with cstr_end...
|
||||
if (*cstr_end == '\0') {
|
||||
// Advance the offset with one extra byte for the NULL terminator
|
||||
*offset_ptr += (cstr_end - cstr + 1);
|
||||
return cstr;
|
||||
}
|
||||
const char *end = reinterpret_cast<const char *>(m_end);
|
||||
|
||||
// We reached the end of the data without finding a NULL C string
|
||||
// terminator. Fall through and return nullptr otherwise anyone that would
|
||||
// have used the result as a C string can wander into unknown memory...
|
||||
}
|
||||
return nullptr;
|
||||
// Check all bytes for a null terminator that terminates a C string.
|
||||
const char *terminator_or_end = std::find(start, end, '\0');
|
||||
|
||||
// We didn't find a null terminator, so return nullptr to indicate that there
|
||||
// is no valid C string at that offset.
|
||||
if (terminator_or_end == end)
|
||||
return nullptr;
|
||||
|
||||
// Update offset_ptr for the caller to point to the data behind the
|
||||
// terminator (which is 1 byte long).
|
||||
*offset_ptr += (terminator_or_end - start + 1UL);
|
||||
return start;
|
||||
}
|
||||
|
||||
// Extracts a NULL terminated C string from the fixed length field of length
|
||||
|
|
|
@ -49,6 +49,51 @@ TEST(DataExtractorTest, PeekData) {
|
|||
EXPECT_EQ(nullptr, E.PeekData(4, 1));
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetCStr) {
|
||||
uint8_t buffer[] = {'X', 'f', 'o', 'o', '\0'};
|
||||
DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
|
||||
|
||||
lldb::offset_t offset = 1;
|
||||
EXPECT_STREQ("foo", E.GetCStr(&offset));
|
||||
EXPECT_EQ(5U, offset);
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetCStrEmpty) {
|
||||
uint8_t buffer[] = {'X', '\0'};
|
||||
DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
|
||||
|
||||
lldb::offset_t offset = 1;
|
||||
EXPECT_STREQ("", E.GetCStr(&offset));
|
||||
EXPECT_EQ(2U, offset);
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetCStrUnterminated) {
|
||||
uint8_t buffer[] = {'X', 'f', 'o', 'o'};
|
||||
DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
|
||||
|
||||
lldb::offset_t offset = 1;
|
||||
EXPECT_EQ(nullptr, E.GetCStr(&offset));
|
||||
EXPECT_EQ(1U, offset);
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetCStrAtEnd) {
|
||||
uint8_t buffer[] = {'X'};
|
||||
DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
|
||||
|
||||
lldb::offset_t offset = 1;
|
||||
EXPECT_EQ(nullptr, E.GetCStr(&offset));
|
||||
EXPECT_EQ(1U, offset);
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetCStrAtNullOffset) {
|
||||
uint8_t buffer[] = {'f', 'o', 'o', '\0'};
|
||||
DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
|
||||
|
||||
lldb::offset_t offset = 0;
|
||||
EXPECT_STREQ("foo", E.GetCStr(&offset));
|
||||
EXPECT_EQ(4U, offset);
|
||||
}
|
||||
|
||||
TEST(DataExtractorTest, GetMaxU64) {
|
||||
uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
|
||||
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
|
||||
|
|
Loading…
Reference in New Issue