https://sourceware.org/bugzilla/show_bug.cgi?id=34873
Bug ID: 34873
Summary: readelf: program and section headers misread when
e_phentsize/e_shentsize is larger than the structure
Product: binutils
Version: 2.48 (HEAD)
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: binutils
Assignee: unassigned at sourceware dot org
Reporter: martin.mirchev at u dot nus.edu
Target Milestone: ---
When e_phentsize is larger than sizeof (ElfXX_External_Phdr), readelf
warns
The e_phentsize field in the ELF header is larger than the size of
an ELF program header
and then carries on. get_data() reads e_phnum entries of e_phentsize
bytes, but ElfXX(_get_program_headers) (readelf-nn.c) walks the buffer
with "external++", i.e. with a stride of sizeof (*phdrs), so every
program header after the first is decoded from the wrong offset.
ElfXX(_get_section_headers) has the same problem: it reads with the
declared e_shentsize but indexes "shdrs[i]".
The ELF spec defines e_phentsize/e_shentsize as the entry size and says
all entries have that size; it does not require it to equal the
structure size. (BFD rejects such files outright; readelf says it
accepts them, so it should use the declared size, or else reject them.)
restride.py (attached) copies an ELF file and re-emits its program and
section header tables at the end of the file with 8 bytes of padding per
entry, updating only e_phoff/e_phentsize and e_shoff/e_shentsize. Both
files should give the same readelf output apart from those fields.
$ echo 'int x = 1; int main (void) { return x; }' > m.c
$ gcc -O2 m.c -o m64-control
$ python3 restride.py m64-control m64-restrided 8
$ readelf -lW m64-control
...
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x0002d8
0x0002d8 R 0x8
INTERP 0x000318 0x0000000000000318 0x0000000000000318 0x00001c
0x00001c R 0x1
[Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x0005c8
0x0005c8 R 0x1000
LOAD 0x001000 0x0000000000001000 0x0000000000001000 0x0001c5
0x0001c5 R E 0x1000
...
$ readelf -lW m64-restrided
readelf: Warning: The e_shentsize field in the ELF header is larger than the
size of an ELF section header
readelf: Warning: out of range sh_link (824) for section 2
readelf: Warning: out of range sh_link (856) for section 3
[... 20 more "out of range" warnings ...]
readelf: Warning: The e_phentsize field in the ELF header is larger than the
size of an ELF program header
Elf file type is DYN (Shared object file)
...
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x0002d8
0x0002d8 R 0x8
NULL 0x400000003 0x0000000000000318 0x0000000000000318 0x000318
0x00001c 0x1c
LOAD 0x000000 0x0000000400000001 0x0000000000000000 0x000000
0x000000 0x5c8
<unknown>: 5c8 0x001000 0x0000000000000000 0x0000000500000001 0x001000
0x001000 0x1000
...
$ readelf -SW m64-restrided
[Nr] Name Type Address Off Size ES Flg
Lk Inf Al
[ 0] ^?ELF^B^A^A NULL 0000000000000000 000000 000000 00
0 0 0
[ 1] ^?ELF^B^A^A NULL 0000000000000002 000318 000318 01
WAxMx 28 0 0
...
(The first entry of each table is right; from the second on, the
fields are shifted by 8 * i bytes. "Shared object file" instead of
"Position-Independent Executable file" is because DT_FLAGS_1 is not
found.) The same happens for a 32-bit (gcc -m32) executable.
Fix (patch attached, against master 0164fa7b33f): address entry i at
(char *) table + i * size in both functions.
--- a/binutils/readelf-nn.c
+++ b/binutils/readelf-nn.c
@@ -47,7 +47,6 @@ static bool
ElfXX(_get_program_headers) (Filedata * filedata, Elf_Internal_Phdr *
pheaders)
{
ElfXX(_External_Phdr) * phdrs;
- const ElfXX(_External_Phdr) * external;
Elf_Internal_Phdr * internal;
unsigned int i;
unsigned int size = filedata->file_header.e_phentsize;
@@ -69,10 +68,16 @@ ElfXX(_get_program_headers) (Filedata * filedata,
Elf_Internal_Phdr * pheaders)
if (phdrs == NULL)
return false;
- for (i = 0, internal = pheaders, external = phdrs;
+ for (i = 0, internal = pheaders;
i < filedata->file_header.e_phnum;
- i++, internal++, external++)
+ i++, internal++)
{
+ /* Entries are e_phentsize bytes apart, which may be more than
+ sizeof (*phdrs). */
+ const ElfXX(_External_Phdr) * external
+ = (const ElfXX(_External_Phdr) *) ((const char *) phdrs
+ + (size_t) i * size);
+
internal->p_type = BYTE_GET (external->p_type);
internal->p_offset = BYTE_GET (external->p_offset);
internal->p_vaddr = BYTE_GET (external->p_vaddr);
@@ -144,16 +149,22 @@ ElfXX(_get_section_headers) (Filedata * filedata, bool
probe)
i < num;
i++, internal++, orig_internal++)
{
- internal->sh_name = BYTE_GET (shdrs[i].sh_name);
- internal->sh_type = BYTE_GET (shdrs[i].sh_type);
[... same replacement of shdrs[i] by external for all ten fields ...]
With the patch, for both the 64-bit and the 32-bit pair, readelf -aW
on the restrided file differs from the control only in the ELF header
fields that were changed and the two "larger than" warnings. readelf
-a -W --debug-dump=rawline,decodedline output is byte-identical before
and after the patch for 784 ordinary ELF files (/usr/bin, system shared
libraries, separate debug files, and the objects of the binutils build).
I could not run the DejaGnu testsuite here.
This issue was found by an LLM-Enhanced Static Analysis Engine and verified
manually; the patch was manually reviewed.
--
You are receiving this mail because:
You are on the CC list for the bug.
