gather.c: fix out of bounds access in maybe_pie() Patch submitted upstream. The crashing function was created here: The out-of-bounds read was first introduced here, but it would ignore program headers at a large offset, so no crashes commonly: --- prelink-cross-20151030/src/gather.c +++ prelink-cross-20151030/src/gather.c @@ -645,59 +645,89 @@ add_dir_to_dirlist (const char *name, de return 0; } -/* Determine if a buffer holding an ELF header and program header - table may be that of a position-independent executable. */ +/* Determine for a buffer holding an ELF header if the program header + table in the buffer or in the associated file may be that of a + position-independent executable. */ static int -maybe_pie (unsigned char *e_ident, int big_endian, int sixty_four) +maybe_pie (unsigned char *e_ident, size_t data_length, int fd, + int big_endian, int sixty_four) { uint16_t num_phdrs; uint16_t phdr; size_t p_type_offset; size_t phnum_offset; + off_t phdr_offset; + size_t phentsize_offset; + uint16_t phentsize; + size_t phdr_size; unsigned char *phdr_table; - unsigned char *this_phdr; + size_t this_offset; + unsigned char buffer[0x1000]; if (sixty_four) { - uint64_t phdr_offset; - + unsigned char *phoff = e_ident + offsetof (Elf64_Ehdr, e_phoff); + phdr_size = sizeof (Elf64_Phdr); p_type_offset = offsetof (Elf64_Phdr, p_type); phnum_offset = offsetof (Elf64_Ehdr, e_phnum); + phentsize_offset = offsetof (Elf64_Ehdr, e_phentsize); if (big_endian) - phdr_offset = buf_read_ube64 (&e_ident [offsetof (Elf64_Ehdr, - e_phoff)]); + phdr_offset = buf_read_ube64 (phoff); else - phdr_offset = buf_read_ule64 (&e_ident [offsetof (Elf64_Ehdr, - e_phoff)]); - phdr_table = e_ident + phdr_offset; + phdr_offset = buf_read_ule64 (phoff); + if (phdr_offset < sizeof (Elf64_Ehdr)) + return 0; } else { - uint32_t phdr_offset; - + unsigned char *phoff = e_ident + offsetof (Elf32_Ehdr, e_phoff); + phdr_size = sizeof (Elf32_Phdr); p_type_offset = offsetof (Elf32_Phdr, p_type); phnum_offset = offsetof (Elf32_Ehdr, e_phnum); + phentsize_offset = offsetof (Elf32_Ehdr, e_phentsize); if (big_endian) - phdr_offset = buf_read_ube32 (&e_ident [offsetof (Elf32_Ehdr, - e_phoff)]); + phdr_offset = buf_read_ube32 (phoff); else - phdr_offset = buf_read_ule32 (&e_ident [offsetof (Elf32_Ehdr, - e_phoff)]); - phdr_table = e_ident + phdr_offset; + phdr_offset = buf_read_ule32 (phoff); + if (phdr_offset < sizeof (Elf32_Ehdr)) + return 0; } - - this_phdr = phdr_table; if (big_endian) - num_phdrs = buf_read_ube16 (&e_ident [phnum_offset]); + { + num_phdrs = buf_read_ube16 (e_ident + phnum_offset); + phentsize = buf_read_ube16 (e_ident + phentsize_offset); + } else - num_phdrs = buf_read_ule16 (&e_ident [phnum_offset]); - - for (phdr = 0; phdr < num_phdrs; phdr++) { - unsigned char *p_type_start = this_phdr + p_type_offset; + num_phdrs = buf_read_ule16 (e_ident + phnum_offset); + phentsize = buf_read_ule16 (e_ident + phentsize_offset); + } + if (num_phdrs == 0 || phentsize < phdr_size) + return 0; + /* TODO: check that phdr_offset + phentsize * num_phdrs doesn't overflow */ + + phdr_table = e_ident; + this_offset = phdr_offset < data_length ? phdr_offset : data_length; + for (phdr = 0; phdr < num_phdrs; phdr++, this_offset += phentsize) + { + unsigned char *p_type_start; uint32_t p_type; - + + if (this_offset + p_type_offset + sizeof (int32_t) > data_length) + { + /* Read more headers from file */ + ssize_t read_bytes = pread (fd, buffer, sizeof(buffer), + phdr_offset + phdr * (off_t) phentsize); + if (read_bytes < phentsize) + return 0; + + data_length = read_bytes; + phdr_table = buffer; + this_offset = 0; + } + + p_type_start = phdr_table + this_offset + p_type_offset; if (big_endian) p_type = buf_read_ube32 (p_type_start); else @@ -709,8 +739,6 @@ maybe_pie (unsigned char *e_ident, int b /* Any PT_PHDR entry must come before any PT_LOAD entry. */ if (p_type == PT_LOAD) return 0; - - this_phdr += sixty_four ? sizeof (Elf64_Phdr) : sizeof (Elf32_Phdr); } return 0; @@ -720,7 +748,7 @@ static int gather_func (const char *name, const struct stat64 *st, int type, struct FTW *ftwp) { - unsigned char e_ident [sizeof (Elf64_Ehdr) + sizeof (Elf64_Phdr)]; + unsigned char e_ident [sizeof (Elf64_Ehdr) + sizeof (Elf64_Phdr) + 4]; #ifndef HAVE_FTW_ACTIONRETVAL if (blacklist_dir) @@ -817,7 +845,7 @@ make_unprelinkable: goto make_unprelinkable; else if (e_ident [EI_CLASS] == ELFCLASS32) { - if (maybe_pie (e_ident, 0, 0)) + if (maybe_pie (e_ident, sizeof (e_ident), fd, 0, 0)) { maybe_pie: dso = fdopen_dso (fd, name); @@ -834,7 +862,7 @@ maybe_pie: } else if (e_ident [EI_CLASS] == ELFCLASS64) { - if (maybe_pie (e_ident, 0, 1)) + if (maybe_pie (e_ident, sizeof (e_ident), fd, 0, 1)) goto maybe_pie; goto close_it; } @@ -851,13 +879,13 @@ maybe_pie: goto make_unprelinkable; else if (e_ident [EI_CLASS] == ELFCLASS32) { - if (maybe_pie (e_ident, 1, 0)) + if (maybe_pie (e_ident, sizeof (e_ident), fd, 1, 0)) goto maybe_pie; goto close_it; } else if (e_ident [EI_CLASS] == ELFCLASS64) { - if (maybe_pie (e_ident, 1, 1)) + if (maybe_pie (e_ident, sizeof (e_ident), fd, 1, 1)) goto maybe_pie; goto close_it; }