From: Mark Wielaard <mjw@redhat.com>
To: elfutils-devel@lists.fedorahosted.org
Subject: Re: out-of-bounds read / crash in elfutils tools (readelf, nm, ...) with malformed file
Date: Thu, 06 Nov 2014 17:05:42 +0100 [thread overview]
Message-ID: <1415289942.19702.22.camel@bordewijk.wildebeest.org> (raw)
In-Reply-To: 1415286703.19702.20.camel@bordewijk.wildebeest.org
[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]
On Thu, 2014-11-06 at 16:11 +0100, Mark Wielaard wrote:
> I haven't figured out yet why we do use the data in the mmap case. It
> looks like we should catch that issue in elf_getdata:
>
> /* We can use the mapped or loaded data if available. */
> if (elf->map_address != NULL)
> {
> /* First see whether the information in the section header is
> valid and it does not ask for too much. */
> if (unlikely (offset + size > elf->maximum_size))
> {
> /* Something is wrong. */
> __libelf_seterrno (ELF_E_INVALID_SECTION_HEADER);
> return 1;
> }
>
> elf->maximum_size is setup correctly based on the actual file size. But
> it seems we don't actually hit this code path in this case. The rawdata
> is setup some other way than by calling __libelf_set_rawdata_wrlock. But
> I haven't figured out how yet.
I believe it comes from a reversed check in elf_begin when it sets up
the base addresses in case the file was mmaped.
diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index c3ad140..5525a3b 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -337,8 +337,8 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
elf->state.elf32.scns.data[cnt].shdr.e32 =
&elf->state.elf32.shdr[cnt];
if (likely (elf->state.elf32.shdr[cnt].sh_offset < maxsize)
- && likely (maxsize - elf->state.elf32.shdr[cnt].sh_offset
- <= elf->state.elf32.shdr[cnt].sh_size))
+ && likely (elf->state.elf32.shdr[cnt].sh_size
+ <= maxsize - elf->state.elf32.shdr[cnt].sh_offset))
elf->state.elf32.scns.data[cnt].rawdata_base =
elf->state.elf32.scns.data[cnt].data_base =
((char *) map_address + offset
@@ -428,8 +428,8 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
elf->state.elf64.scns.data[cnt].shdr.e64 =
&elf->state.elf64.shdr[cnt];
if (likely (elf->state.elf64.shdr[cnt].sh_offset < maxsize)
- && likely (maxsize - elf->state.elf64.shdr[cnt].sh_offset
- <= elf->state.elf64.shdr[cnt].sh_size))
+ && likely (elf->state.elf64.shdr[cnt].sh_size
+ <= maxsize - elf->state.elf64.shdr[cnt].sh_offset))
elf->state.elf64.scns.data[cnt].rawdata_base =
elf->state.elf64.scns.data[cnt].data_base =
((char *) map_address + offset
With that the invalid sized sections aren't precached anymore and so
rawdata will remain NULL. __libelf_set_rawdata_wrlock will be called and
readelf will just report "cannot get data for section [28] '.strtab':
invalid section header" instead of trying to use the data.
Does the above look sane? All tests PASS with the above change.
Thanks,
Mark
next reply other threads:[~2014-11-06 16:05 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-06 16:05 Mark Wielaard [this message]
-- strict thread matches above, loose matches on Subject: below --
2014-11-13 21:55
2014-11-13 21:51 Mark Wielaard
2014-11-13 19:39
2014-11-13 14:45 Mark Wielaard
2014-11-11 16:57 Mark Wielaard
2014-11-11 13:57
2014-11-11 13:53 Mark Wielaard
2014-11-11 13:49 Petr Machata
2014-11-11 13:40
2014-11-11 13:30 Petr Machata
2014-11-11 13:15 Mark Wielaard
2014-11-11 10:31
2014-11-10 20:58 Mark Wielaard
2014-11-09 21:59
2014-11-09 16:57 Mark Wielaard
2014-11-08 16:10
2014-11-08 15:32 Mark Wielaard
2014-11-08 14:04 Mark Wielaard
2014-11-07 16:13
2014-11-07 15:45 Mark Wielaard
2014-11-07 15:32
2014-11-07 11:58 Mark Wielaard
2014-11-07 11:51 Mark Wielaard
2014-11-07 0:27
2014-11-06 18:25 Roland McGrath
2014-11-06 15:11 Mark Wielaard
2014-10-31 16:13
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1415289942.19702.22.camel@bordewijk.wildebeest.org \
--to=mjw@redhat.com \
--cc=elfutils-devel@lists.fedorahosted.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).