This is the mail archive of the mailing list for the elfutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Make gelf_getphdr more robust?

On 02/06/2014 02:20 PM, Mark Wielaard wrote:

>> The check against maxsize is insufficient, it's also required to check
>> that phdr->p_filesz <= maxsize - phdr->p_offset.  Would it make sense to
>> do both checks inside gelf_getphdr?
> I think this case is actually caught now by default by the integrated
> robustify patches from last month, specifically:
> + /* First see whether the information in the ELF header is
> + valid and it does not ask for too much. */
> + if (unlikely (ehdr->e_phoff >= elf->maximum_size)
> +     || unlikely (elf->maximum_size - ehdr->e_phoff < size))
> + {
> +   /* Something is wrong. */
> +   __libelf_seterrno (ELF_E_INVALID_PHDR);
> +   goto out;
> + }
> So that would make the maxsize check in readelf.c redundant.

Looks like this.  I was testing the version in Fedora 20, 

>> And the printf call can leak data or crash if filedata + phdr->p_offset
>> is not NUL terminated (which obviously needs a crafted ELF file).  This
>> can't be fixed in gelf_getphdr, alas.
> Yes, it would probably be good to catch this in eu-readelf.
> Did you catch this by code inspection or did you stumble upon a bad ELF
> file that made it crash?

I noticed this issue in the source code when I ivestigated why my own 
code complained about about a malformed interp header in a debuginfo 
package.  I have built a crafted ELF image derived from /usr/bin/gs that 
crashes eu-readelf (running under valgrind), but it's nothing that I 
found in the wild.

Florian Weimer / Red Hat Product Security Team

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]