In function 'parse_die' there is an undefined behavior in expressions like xptr + block_len < xptr. Due to variable 'block_len' being unsigned integer, such expressions may be completely omitted by compiler as demonstrated by the following proof-of-concept: The function int test(char *p, unsigned int sz) { return p + sz < p; } may be turned into the following assembly code .file "test.c" .text .p2align 4,,15 .globl test .type test, @function test: .LFB0: .cfi_startproc xorl %eax, %eax ret .cfi_endproc .LFE0: .size test, .-test .ident "GCC: (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0" .section .note.GNU-stack,"",@progbits by the command gcc -O2 -S -o- test.c So the external function 'test' always returns 0. To be precise, the issues comes in the code fragments labeled with 'FORM_BLOCK2' and 'FORM_BLOCK4'.
I've verified the case by performing these steps: 1) Configure the project: binutils-gdb$ ./configure CFLAGS='-g -O2' CXXFLAGS='-g -O2' LDFLAGS='-g -O2' host_configargs='--disable-option-checking --disable-silent-rules' 2) Observe compilation command for dwarf1.c; slightly modify it to produce assembly listing instead: binutils-gdb/bfd$ gcc -DHAVE_CONFIG_H -I. -DBINDIR=\"/usr/local/bin\" -DLIBDIR=\"/usr/local/lib\" -I. -I. -I./../include -DHAVE_x86_64_elf64_vec -DHAVE_i386_elf32_vec -DHAVE_iamcu_elf32_vec -DHAVE_x86_64_elf32_vec -DHAVE_i386_pei_vec -DHAVE_x86_64_pe_vec -DHAVE_x86_64_pei_vec -DHAVE_l1om_elf64_vec -DHAVE_k1om_elf64_vec -DHAVE_elf64_le_vec -DHAVE_elf64_be_vec -DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -I./../zlib -g -O2 -S dwarf1.c -o- 3) The C fragment 268: case FORM_BLOCK4: 269: if (xptr + 4 <= aDiePtrEnd) 270: { 271: block_len = bfd_get_32 (abfd, xptr); 272: if (xptr + block_len > aDiePtrEnd 273: || xptr + block_len < xptr) 274: return false; 275: xptr += block_len; 276: } 277: xptr += 4; 278: break; is turned into .LVL22: .loc 1 272 0 movl %eax, %eax addq %rax, %r14 .LVL23: cmpq %r14, %r12 jb .L4 .LVL24: .L19: .loc 1 277 0 leaq 4(%r14), %rbx by gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04) Note that the condition 'xptr + block_len < xptr' is completely omitted as is clear from the assembly location markers.
Created attachment 13857 [details] PATCH: Switch to a pointer difference in comparisons I'm submitting a proposed patch to prevent that case.
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4d3605c8ca92bcde848581a8ec031827c798501b commit 4d3605c8ca92bcde848581a8ec031827c798501b Author: Nikita Popov <npv1310@gmail.com> Date: Wed Dec 15 17:49:06 2021 +0000 Fix an undefined behaviour in the BFD library's DWARF parser. PR 28687 * dwarf1.c (parse_die): Fix undefined behaviour in range tests.
Hi Nikita, Thanks for reporting this problem - and providing a fix! I have now checked in your patch. Cheers Nick
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=05f62e0c9a0b14e211c6b2b6234095b50794b20b commit 05f62e0c9a0b14e211c6b2b6234095b50794b20b Author: Alan Modra <amodra@gmail.com> Date: Thu Dec 16 10:50:58 2021 +1030 Re: Fix an undefined behaviour in the BFD library's DWARF parser Using an unsigned int cast (to 32 bits) on a pointer difference (of possibly 64 bits) is wrong. Even though it will work on all real object files, the fuzzers will eventually find this hole. PR 28687 * dwarf1.c (parse_die): Cast pointer difference to size_t. Catch another possible pointer overflow.