zdump/dfi_elf: Fix stack buffer overflow in nt_read()

Sanity check ELF notes descriptor size before reading it. This prevents
stack buffer overflows in case a dump contains invalid ELF notes.
Instead of reading a note's descriptor into a temporary buffer on stack,
read it directly into the buffer given to nt_read() but also provide
a maximum length of the given buffer to nt_read() in order to prevent
overflows.

This problem was found with valgrind and AFL fuzzing + ASAN.

AFL +  ASAN findings:

[root@t83lp49 s390-tools]# ./zdump/zgetdump -iVVVVV ~/zgetdump-fuzzing/findings/crashes/id\:000008\,sig\:06\,src\:000007\,op\:arith8\,pos\:67\,val\:+3
TRACE: DFI initialization
DEBUG: DFI trying s390tape
DEBUG: DFI s390tape returned with rc -19
DEBUG: DFI trying devmem
DEBUG: DFI devmem returned with rc -19
DEBUG: DFI trying s390mv_ext
DEBUG: DFI s390mv_ext returned with rc -19
DEBUG: DFI trying s390mv
DEBUG: DFI s390mv returned with rc -19
DEBUG: DFI trying s390_ext
DEBUG: DFI S390 extended initialization
DEBUG: DFI s390_ext returned with rc -19
DEBUG: DFI trying s390
DEBUG: DFI S390 initialization
DEBUG: DFI s390 returned with rc -19
DEBUG: DFI trying lkcd
DEBUG: DFI lkcd returned with rc -19
DEBUG: DFI trying elf
DEBUG: DFI ELF initialization
DEBUG: DFI ELF e_phnum 11
DEBUG: DFI ELF p_type[0] 0x4
DEBUG: DFI ELF n_type 0x0
AddressSanitizer:DEADLYSIGNAL
=================================================================
==208548==ERROR: AddressSanitizer: stack-overflow on address 0x03ffef05d000 (pc 0x0000010051b0 bp 0x03fff107dc40 sp 0x03ffef05dac8 T0)
    #0 0x10051b0  (/root/s390-tools/zdump/zgetdump+0x10051b0)

SUMMARY: AddressSanitizer: stack-overflow (/root/s390-tools/zdump/zgetdump+0x10051b0)
==208548==ABORTING

valgrind findings:

	==56423== Source and destination overlap in memcpy(0x4a86d38, 0x4a875e0, 4096)
	==56423==    at 0x4839F86: memcpy (in /usr/lib/s390x-linux-gnu/valgrind/vgpreload_memcheck-s390x-linux.so)
	==56423==    by 0x114253: memcpy (string_fortified.h:34)
	==56423==    by 0x114253: nt_read (dfi_elf.c:95)
	==56423==    by 0x1145FF: nt_s390_prefix_read (dfi_elf.c:195)
	==56423==    by 0x1145FF: pt_notes_add (dfi_elf.c:259)
	==56423==    by 0x1145FF: dfi_elf_init (dfi_elf.c:326)
	==56423==    by 0x112A57: dfi_init (dfi.c:1212)
	==56423==    by 0x10D663: do_dump_info (zgetdump.c:127)
	==56423==    by 0x10D663: main (zgetdump.c:182)
	==56423==
	==56423== Invalid write of size 8
	==56423==    at 0x4839E28: memcpy (in /usr/lib/s390x-linux-gnu/valgrind/vgpreload_memcheck-s390x-linux.so)
	==56423==    by 0x114253: memcpy (string_fortified.h:34)
	==56423==    by 0x114253: nt_read (dfi_elf.c:95)
	==56423==    by 0x1145FF: nt_s390_prefix_read (dfi_elf.c:195)
	==56423==    by 0x1145FF: pt_notes_add (dfi_elf.c:259)
	==56423==    by 0x1145FF: dfi_elf_init (dfi_elf.c:326)
	==56423==    by 0x112A57: dfi_init (dfi.c:1212)
	==56423==    by 0x10D663: do_dump_info (zgetdump.c:127)
	==56423==    by 0x10D663: main (zgetdump.c:182)
	==56423==  Address 0x4a86ee0 is 0 bytes after a block of size 912 alloc'd
	==56423==    at 0x483675E: calloc (in /usr/lib/s390x-linux-gnu/valgrind/vgpreload_memcheck-s390x-linux.so)
	==56423==    by 0x10E71D: zg_alloc (zg.c:93)
	==56423==    by 0x114793: nt_prstatus_read (dfi_elf.c:123)
	==56423==    by 0x114793: pt_notes_add (dfi_elf.c:234)
	==56423==    by 0x114793: dfi_elf_init (dfi_elf.c:326)
	==56423==    by 0x112A57: dfi_init (dfi.c:1212)
	==56423==    by 0x10D663: do_dump_info (zgetdump.c:127)
	==56423==    by 0x10D663: main (zgetdump.c:182)

Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Suggested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reported-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
This commit is contained in:
Alexander Egorenkov
2021-09-03 22:03:33 +02:00
committed by Jan Höppner
parent 28db3523d0
commit cb09223a56

View File

@@ -70,38 +70,36 @@ static int pt_load_add(Elf64_Phdr *phdr)
return 0;
}
/*
* Skip name of note
*/
static void nt_name_skip(Elf64_Nhdr *note)
{
zg_seek_cur(g.fh, ROUNDUP(note->n_namesz, 4), ZG_CHECK);
}
/*
* Read note
*/
static int nt_read(Elf64_Nhdr *note, void *buf)
static int nt_read(const Elf64_Nhdr *note, void *buf, size_t buf_len)
{
off_t buf_len = ROUNDUP(note->n_descsz, 4);
char tmp_buf[buf_len];
ssize_t nread;
util_log_print(UTIL_LOG_TRACE, "DFI ELF n_descsz %u\n", buf_len);
nt_name_skip(note);
if (zg_read(g.fh, tmp_buf, buf_len, ZG_CHECK_ERR) != buf_len)
/* We cannot read more than the current note provides */
if (note->n_descsz < buf_len)
return -EINVAL;
if (buf)
memcpy(buf, tmp_buf, note->n_descsz);
/* Skip note's name and position file at note's descriptor */
zg_seek_cur(g.fh, ROUNDUP(note->n_namesz, 4), ZG_CHECK);
/* Read note's descriptor */
nread = zg_read(g.fh, buf, buf_len, ZG_CHECK_ERR);
if (nread < 0 || (size_t)nread != buf_len)
return -EINVAL;
/* Skip the rest of note's descriptor until the next note */
zg_seek_cur(g.fh, ROUNDUP(note->n_descsz, 4) - buf_len, ZG_CHECK);
return 0;
}
/*
* Skip note
*/
static int nt_skip(Elf64_Nhdr *note)
static void nt_skip(const Elf64_Nhdr *note)
{
return nt_read(note, NULL);
/* Skip note's name + descriptor and position file at the next note */
zg_seek_cur(g.fh,
ROUNDUP(note->n_namesz, 4) + ROUNDUP(note->n_descsz, 4),
ZG_CHECK);
}
/*
@@ -122,7 +120,7 @@ static struct dfi_cpu *nt_prstatus_read(Elf64_Nhdr *note)
struct dfi_cpu *cpu = dfi_cpu_alloc();
struct nt_prstatus_64 nt_prstatus;
if (nt_read(note, &nt_prstatus))
if (nt_read(note, &nt_prstatus, sizeof(nt_prstatus)))
return NULL;
memcpy(cpu->gprs, &nt_prstatus.gprs, sizeof(cpu->gprs));
@@ -141,7 +139,7 @@ static int nt_fpregset_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
struct nt_fpregset_64 nt_fpregset;
check_cpu(cpu, "FPREGSET");
if (nt_read(note, &nt_fpregset))
if (nt_read(note, &nt_fpregset, sizeof(nt_fpregset)))
return -EINVAL;
memcpy(&cpu->fpc, &nt_fpregset.fpc, sizeof(cpu->fpc));
@@ -155,7 +153,7 @@ static int nt_fpregset_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_timer_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_TIMER");
return nt_read(note, &cpu->timer);
return nt_read(note, &cpu->timer, sizeof(cpu->timer));
}
/*
@@ -164,7 +162,7 @@ static int nt_s390_timer_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_todcmp_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_TODCMP");
return nt_read(note, &cpu->todcmp);
return nt_read(note, &cpu->todcmp, sizeof(cpu->todcmp));
}
/*
@@ -173,7 +171,7 @@ static int nt_s390_todcmp_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_todpreg_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_TODPREG");
return nt_read(note, &cpu->todpreg);
return nt_read(note, &cpu->todpreg, sizeof(cpu->todpreg));
}
/*
@@ -182,7 +180,7 @@ static int nt_s390_todpreg_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_ctrs_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_CTRS");
return nt_read(note, &cpu->ctrs);
return nt_read(note, &cpu->ctrs, sizeof(cpu->ctrs));
}
/*
@@ -191,7 +189,7 @@ static int nt_s390_ctrs_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_prefix_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_PREFIX");
return nt_read(note, &cpu->prefix);
return nt_read(note, &cpu->prefix, sizeof(cpu->prefix));
}
/*
@@ -200,7 +198,7 @@ static int nt_s390_prefix_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_vxrs_low_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_VXRS_LOW");
return nt_read(note, &cpu->vxrs_low);
return nt_read(note, &cpu->vxrs_low, sizeof(cpu->vxrs_low));
}
/*
@@ -209,7 +207,7 @@ static int nt_s390_vxrs_low_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
static int nt_s390_vxrs_high_read(struct dfi_cpu *cpu, Elf64_Nhdr *note)
{
check_cpu(cpu, "S390_VXRS_HIGH");
return nt_read(note, &cpu->vxrs_high);
return nt_read(note, &cpu->vxrs_high, sizeof(cpu->vxrs_high));
}
/*
@@ -271,8 +269,7 @@ static int pt_notes_add(Elf64_Phdr *phdr)
dfi_cpu_content_fac_add(DFI_CPU_CONTENT_FAC_VX);
break;
default:
if (nt_skip(&note))
return -EINVAL;
nt_skip(&note);
break;
}
}