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:
https://git.fedorahosted.org/cgit/elfutils.git/commit/?id=720383c53b435de6647edd78060dd7d38ade25a5
+ /* 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,
elfutils-0.158-1.fc20.x86_64.
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
