[Bug 43625] New: DataExtractor::GetCStr may access extra byte out of bound when working with non-zero terminated string

Bug ID 43625
Summary DataExtractor::GetCStr may access extra byte out of bound when working with non-zero terminated string
Product lldb
Version unspecified
Hardware All
OS All
Status NEW
Severity normal
Priority P
Component All Bugs
Assignee lldb-dev@lists.llvm.org
Reporter vtolkov@gmail.com
CC jdevlieghere@apple.com, llvm-bugs@lists.llvm.org

864     const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
865       const char *cstr = (const char *)PeekData(*offset_ptr, 1);
866       if (cstr) {
867         const char *cstr_end = cstr;
868         const char *end = (const char *)m_end;
869         while (cstr_end < end && *cstr_end)
870           ++cstr_end;
871     
872         // Now we are either at the end of the data or we point to the
873         // NULL C string terminator with cstr_end...

At this point we have loop exit condition: (cstr_end>=end || *cstr_end==0).
If we've reached the end, we shouldn't test (*cstr_end), it is beyond the
limit.
So instead of

874         if (*cstr_end == '\0') {

it should be:

            if (cstr_end < end) {

and the rest of function are here just for completeness:

875           // Advance the offset with one extra byte for the NULL terminator
876           *offset_ptr += (cstr_end - cstr + 1);
877           return cstr;
878         }
879     
880         // We reached the end of the data without finding a NULL C string
881         // terminator. Fall through and return nullptr otherwise anyone
that would
882         // have used the result as a C string can wander into unknown
memory...
883       }
884       return nullptr;
885     }

Raphael Isemann changed bug 43625