* [PATCH] strings: Produce error when section data falls outside file.
@ 2014-12-26 22:00 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2014-12-26 22:00 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]
https://bugzilla.redhat.com/show_bug.cgi?id=1170810
Reported-by: Alexander Cherepanov <cherepan@mccme.ru>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
src/ChangeLog | 5 +++++
src/strings.c | 17 +++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 904b3c9..66d6270 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2014-12-26 Mark Wielaard <mjw@redhat.com>
+ * strings.c (read_elf): Produce error when section data falls outside
+ file.
+
+2014-12-26 Mark Wielaard <mjw@redhat.com>
+
* nm.c (show_symbols): Guard against divide by zero in error check.
Add section index number in error message.
diff --git a/src/strings.c b/src/strings.c
index f60e4b4..d1eb7b2 100644
--- a/src/strings.c
+++ b/src/strings.c
@@ -725,8 +725,21 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
actually have content. */
if (shdr != NULL && shdr->sh_type != SHT_NOBITS
&& (shdr->sh_flags & SHF_ALLOC) != 0)
- result |= read_block (fd, fname, fdlen, shdr->sh_offset,
- shdr->sh_offset + shdr->sh_size);
+ {
+ if (shdr->sh_offset > fdlen
+ || fdlen - shdr->sh_offset < shdr->sh_size)
+ {
+ size_t strndx = 0;
+ elf_getshdrstrndx (elf, &strndx);
+ error (0, 0,
+ gettext ("Skipping section %zd '%s' data outside file"),
+ elf_ndxscn (scn), elf_strptr (elf, strndx, shdr->sh_name));
+ result = 1;
+ }
+ else
+ result |= read_block (fd, fname, fdlen, shdr->sh_offset,
+ shdr->sh_offset + shdr->sh_size);
+ }
}
while ((scn = elf_nextscn (elf, scn)) != NULL);
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] strings: Produce error when section data falls outside file.
@ 2015-01-12 21:11 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2015-01-12 21:11 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 752 bytes --]
On Mon, 2014-12-29 at 01:15 +0300, Alexander Cherepanov wrote:
> On 2014-12-27 14:42, Mark Wielaard wrote:
> > On Sat, Dec 27, 2014 at 04:31:14AM +0300, Alexander Cherepanov wrote:
> >> strings.c: In function 'read_elf':
> >> strings.c:729:24: error: comparison between signed and unsigned integer
> >> expressions [-Werror=sign-compare]
> >> if (shdr->sh_offset > fdlen
> >
> > The compiler is correct. sh_offset is an unsigned Elf64_Off and fdlen is
> > a signed off64_t. But just before we already checked fdlen >= 0, so we
>
> I see.
>
> > can just cast it to Elf64_Off safely for this comparison. As attached.
>
> Thanks, mjw/pending is compiling fine now.
Thanks for double checking. I pushed that patch to master now.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] strings: Produce error when section data falls outside file.
@ 2014-12-28 22:15 Alexander Cherepanov
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Cherepanov @ 2014-12-28 22:15 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]
On 2014-12-27 14:42, Mark Wielaard wrote:
> On Sat, Dec 27, 2014 at 04:31:14AM +0300, Alexander Cherepanov wrote:
>> On 2014-12-27 01:00, Mark Wielaard wrote:
>>> diff --git a/src/strings.c b/src/strings.c
>>> index f60e4b4..d1eb7b2 100644
>>> --- a/src/strings.c
>>> +++ b/src/strings.c
>>> @@ -725,8 +725,21 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
>>> actually have content. */
>>> if (shdr != NULL && shdr->sh_type != SHT_NOBITS
>>> && (shdr->sh_flags & SHF_ALLOC) != 0)
>>> - result |= read_block (fd, fname, fdlen, shdr->sh_offset,
>>> - shdr->sh_offset + shdr->sh_size);
>>> + {
>>> + if (shdr->sh_offset > fdlen
>>> + || fdlen - shdr->sh_offset < shdr->sh_size)
>>> + {
>>
>> It fails to build for me:
>>
>> strings.c: In function 'read_elf':
>> strings.c:729:24: error: comparison between signed and unsigned integer
>> expressions [-Werror=sign-compare]
>> if (shdr->sh_offset > fdlen
>
> The compiler is correct. sh_offset is an unsigned Elf64_Off and fdlen is
> a signed off64_t. But just before we already checked fdlen >= 0, so we
I see.
> can just cast it to Elf64_Off safely for this comparison. As attached.
Thanks, mjw/pending is compiling fine now.
--
Alexander Cherepanov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] strings: Produce error when section data falls outside file.
@ 2014-12-27 11:42 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2014-12-27 11:42 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]
On Sat, Dec 27, 2014 at 04:31:14AM +0300, Alexander Cherepanov wrote:
> On 2014-12-27 01:00, Mark Wielaard wrote:
> >diff --git a/src/strings.c b/src/strings.c
> >index f60e4b4..d1eb7b2 100644
> >--- a/src/strings.c
> >+++ b/src/strings.c
> >@@ -725,8 +725,21 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
> > actually have content. */
> > if (shdr != NULL && shdr->sh_type != SHT_NOBITS
> > && (shdr->sh_flags & SHF_ALLOC) != 0)
> >- result |= read_block (fd, fname, fdlen, shdr->sh_offset,
> >- shdr->sh_offset + shdr->sh_size);
> >+ {
> >+ if (shdr->sh_offset > fdlen
> >+ || fdlen - shdr->sh_offset < shdr->sh_size)
> >+ {
>
> It fails to build for me:
>
> strings.c: In function 'read_elf':
> strings.c:729:24: error: comparison between signed and unsigned integer
> expressions [-Werror=sign-compare]
> if (shdr->sh_offset > fdlen
The compiler is correct. sh_offset is an unsigned Elf64_Off and fdlen is
a signed off64_t. But just before we already checked fdlen >= 0, so we
can just cast it to Elf64_Off safely for this comparison. As attached.
Thanks,
Mark
[-- Attachment #2: 0001-strings-Produce-error-when-section-data-falls-outsid.patch --]
[-- Type: text/plain, Size: 1895 bytes --]
>From 37497cb28139057a5413f9592b6ceb3a06c41482 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 26 Dec 2014 22:57:00 +0100
Subject: [PATCH] strings: Produce error when section data falls outside file.
https://bugzilla.redhat.com/show_bug.cgi?id=1170810
Reported-by: Alexander Cherepanov <cherepan@mccme.ru>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
src/ChangeLog | 5 +++++
src/strings.c | 17 +++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 904b3c9..66d6270 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2014-12-26 Mark Wielaard <mjw@redhat.com>
+ * strings.c (read_elf): Produce error when section data falls outside
+ file.
+
+2014-12-26 Mark Wielaard <mjw@redhat.com>
+
* nm.c (show_symbols): Guard against divide by zero in error check.
Add section index number in error message.
diff --git a/src/strings.c b/src/strings.c
index f60e4b4..b2bce7b 100644
--- a/src/strings.c
+++ b/src/strings.c
@@ -725,8 +725,21 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
actually have content. */
if (shdr != NULL && shdr->sh_type != SHT_NOBITS
&& (shdr->sh_flags & SHF_ALLOC) != 0)
- result |= read_block (fd, fname, fdlen, shdr->sh_offset,
- shdr->sh_offset + shdr->sh_size);
+ {
+ if (shdr->sh_offset > (Elf64_Off) fdlen
+ || fdlen - shdr->sh_offset < shdr->sh_size)
+ {
+ size_t strndx = 0;
+ elf_getshdrstrndx (elf, &strndx);
+ error (0, 0,
+ gettext ("Skipping section %zd '%s' data outside file"),
+ elf_ndxscn (scn), elf_strptr (elf, strndx, shdr->sh_name));
+ result = 1;
+ }
+ else
+ result |= read_block (fd, fname, fdlen, shdr->sh_offset,
+ shdr->sh_offset + shdr->sh_size);
+ }
}
while ((scn = elf_nextscn (elf, scn)) != NULL);
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] strings: Produce error when section data falls outside file.
@ 2014-12-27 1:31 Alexander Cherepanov
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Cherepanov @ 2014-12-27 1:31 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On 2014-12-27 01:00, Mark Wielaard wrote:
> diff --git a/src/strings.c b/src/strings.c
> index f60e4b4..d1eb7b2 100644
> --- a/src/strings.c
> +++ b/src/strings.c
> @@ -725,8 +725,21 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
> actually have content. */
> if (shdr != NULL && shdr->sh_type != SHT_NOBITS
> && (shdr->sh_flags & SHF_ALLOC) != 0)
> - result |= read_block (fd, fname, fdlen, shdr->sh_offset,
> - shdr->sh_offset + shdr->sh_size);
> + {
> + if (shdr->sh_offset > fdlen
> + || fdlen - shdr->sh_offset < shdr->sh_size)
> + {
It fails to build for me:
strings.c: In function 'read_elf':
strings.c:729:24: error: comparison between signed and unsigned integer
expressions [-Werror=sign-compare]
if (shdr->sh_offset > fdlen
^
cc1: all warnings being treated as errors
--
Alexander Cherepanov
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-12 21:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-26 22:00 [PATCH] strings: Produce error when section data falls outside file Mark Wielaard
2014-12-27 1:31 Alexander Cherepanov
2014-12-27 11:42 Mark Wielaard
2014-12-28 22:15 Alexander Cherepanov
2015-01-12 21:11 Mark Wielaard
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).