[Rpm-maint] [PATCH] debugedit: Check .debug_str index is valid before use.

[Panu Matilainen]

On 03/09/2018 04:24 PM, Panu Matilainen wrote:
> On 03/07/2018 05:25 PM, Mark Wielaard wrote:
>> debugedit would blindly use an .debug_str index from the .debug_info or
>> .debug_line sections assuming it would result in a valid string. Which
>> would crash and burn if the DWARF data was bogus when the string was
>> used. So check whenever converting an string index into a char pointer
>> so we can produce a more helpful error message.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1543912
>>
>> Signed-off-by: Mark Wielaard <mark at klomp.org>
>> ---
>>   tools/debugedit.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/tools/debugedit.c b/tools/debugedit.c
>> index 57cd830..6c71cbc 100644
>> --- a/tools/debugedit.c
>> +++ b/tools/debugedit.c
>> @@ -820,6 +820,9 @@ record_file_string_entry_idx (struct strings 
>> *strings, size_t old_idx)
>>     struct stridxentry *entry = string_find_new_entry (strings, old_idx);
>>     if (entry != NULL)
>>       {
>> +      if (old_idx >= debug_sections[DEBUG_STR].size)
>> +    error (1, 0, "Bad string pointer index %zd", old_idx);
>> +
>>         Strent *strent;
>>         const char *old_str = (char *)debug_sections[DEBUG_STR].data + 
>> old_idx;
>>         const char *file = skip_dir_prefix (old_str, base_dir);
>> @@ -870,6 +873,9 @@ record_existing_string_entry_idx (struct strings 
>> *strings, size_t old_idx)
>>     struct stridxentry *entry = string_find_new_entry (strings, old_idx);
>>     if (entry != NULL)
>>       {
>> +      if (old_idx >= debug_sections[DEBUG_STR].size)
>> +    error (1, 0, "Bad string pointer index %zd", old_idx);
>> +
>>         const char *str = (char *)debug_sections[DEBUG_STR].data + 
>> old_idx;
>>         Strent *strent = strtab_add_len (strings->str_tab,
>>                          str, strlen (str) + 1);
>> @@ -1533,6 +1539,10 @@ edit_attributes (DSO *dso, unsigned char *ptr, 
>> struct abbrev_tag *t, int phase)
>>           {
>>             const char *dir;
>>             size_t idx = do_read_32_relocated (ptr);
>> +          if (idx >= debug_sections[DEBUG_STR].size)
>> +            error (1, 0,
>> +               "%s: Bad string pointer index %zd for comp_dir",
>> +               dso->filename, idx);
>>             dir = (char *) debug_sections[DEBUG_STR].data + idx;
>>             free (comp_dir);
>> @@ -1558,6 +1568,10 @@ edit_attributes (DSO *dso, unsigned char *ptr, 
>> struct abbrev_tag *t, int phase)
>>            case.  */
>>             char *name;
>>             size_t idx = do_read_32_relocated (ptr);
>> +          if (idx >= debug_sections[DEBUG_STR].size)
>> +        error (1, 0,
>> +               "%s: Bad string pointer index %zd for unit name",
>> +               dso->filename, idx);
>>             name = (char *) debug_sections[DEBUG_STR].data + idx;
>>             if (*name == '/' && comp_dir == NULL)
>>           {
>>
> 
> I was somewhat tempted to mumble something about putting these 
> common-looking checks into a function but ... meh, it's Friday :P
> 
> Applied, thanks for the patch!

Actually, this breaks a bunch of testcases:

139: rpmbuild debuginfo subpackages multiple         FAILED 
(rpmbuild.at:973)
140: rpmbuild debuginfo subpackages multiple unique  FAILED 
(rpmbuild.at:1058)
141: rpmbuild debuginfo subpackages multiple unique debugsource FAILED 
(rpmbuild.at:1143)
142: rpmbuild debuginfo subpackages multiple excluded FAILED 
(rpmbuild.at:1231)
143: rpmbuild debuginfo subpackages multiple excluded FAILED 
(rpmbuild.at:1296)

They're all failing with messages like this:

/home/pmatilai/repos/rpm/tests/testing/usr/lib/rpm/debugedit: 
/home/pmatilai/repos/rpm/tests/testing/build/BUILDROOT/test-1.0-1.x86_64/bin/hello2: 
Bad string pointer index 678 for unit name

	- Panu -