Re: [PATCH v2] elf-parse: avoid an undefined shift when checking the ELF type

[Nathan Chancellor]

> elf_parse() takes a bitmap of permitted ELF types and tests it with
> 
> 	type = elf_parser.r2(&ehdr->e32.e_type);
> 	if (!((1 << type) & types)) {
> 
> e_type is a 16-bit field read straight out of the file, so a type of 32
> or more shifts by at least the width of int and the result is undefined;
> 31 shifts into the sign bit. All three users of the parser -- kallsyms,
> sorttable and tracepoint-update -- reach this with e_type unvalidated.
> 
> Found by fuzzing the host ELF tools with mutated object files under
> ASan/UBSan. Reproducer, on any vmlinux (or any ET_REL object for
> tracepoint-update), setting only e_type:
> 
>   $ python3 -c 'import struct
>   d = bytearray(open("vmlinux","rb").read())
>   struct.pack_into("<H", d, 0x10, 40)
>   open("t.elf","wb").write(bytes(d))'
> 
>   $ ./kallsyms --sysmap=/dev/null t.elf
>   scripts/elf-parse.c:94:11: runtime error: shift exponent 40 is too
>   large for 32-bit type 'int'
>       #0 in elf_parse scripts/elf-parse.c:94
> 
> (host tools built with -fsanitize=address,undefined)
> 
> This does not need a corrupt file. include/uapi/linux/elf.h itself
> declares legitimate types above 32:
> 
> 	#define ET_LOPROC 0xff00
> 	#define ET_HIPROC 0xffff
> 
> so any processor-specific object -- and the 0xfe00..0xfeff OS-specific
> range the ELF spec reserves alongside it -- shifts out of range here
> while being a perfectly well-formed file. The tools would reject such an
> object either way; the point is that they currently reach that decision
> through undefined behaviour.
> 
> Reject a type that cannot be in the bitmap, and shift an unsigned 1 so
> that a type of 31 stays defined. No valid object changes behaviour: every
> ET_* value the callers put in the bitmap is far below 32, and a file with
> a larger e_type already took the error path -- it just took it through
> undefined behaviour.
> 
> Fixes: b055f4c431e3 ("sorttable: Move ELF parsing into scripts/elf-parse.[ch]")
> Assisted-by: LLM ASan UBSan
> Signed-off-by: Leizhen Zhang <lzsx618@xxxxxxxxx>

While I am not the biggest fan of these "handle unlikely invalid input",
this seems like reasonable enough hardening.

Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>

-- 
Cheers,
Nathan