Bug #10505
closedelfexec() should keep to unsigned types when processing PT_DYNAMIC
100%
Description
As reported at omniosorg/illumos-omnio#394 , it is possible to crash a system by modifying a binary to have an extremely large p_filesz
value in its PT_DYNAMIC
section. This field is unsigned, but it is then changed to a signed value in several places:
off_t i = 0; #define DYN_STRIDE 100 for (i = 0; i < dynamicphdr->p_filesz; i += sizeof (*dyn) * DYN_STRIDE) { int ndyns = (dynamicphdr->p_filesz - i) / sizeof (*dyn); size_t dynsize; ndyns = MIN(DYN_STRIDE, ndyns); dynsize = ndyns * sizeof (*dyn); dyn = kmem_alloc(dynsize, KM_SLEEP); if ((error = vn_rdwr(UIO_READ, vp, (caddr_t)dyn, dynsize, (offset_t)(dynamicphdr->p_offset + i), UIO_SYSSPACE, 0, (rlim64_t)0, CRED(), &resid)) != 0) { uprintf("%s: cannot read .dynamic section\n", exec_file); goto out; } for (dp = dyn; dp < (dyn + ndyns); dp++) { if (dp->d_tag == DT_SUNW_ASLR) { if ((error = handle_secflag_dt(p, DT_SUNW_ASLR, dp->d_un.d_val)) != 0) { uprintf("%s: error setting " "security-flag from " "DT_SUNW_ASLR: %d\n", exec_file, error); goto out; } } } kmem_free(dyn, dynsize); }
The type of i
is off_t
, which is signed, which is a problem for the loop's condition. The type of ndyns
is int
, which is also signed, which results in a negative value, which survives the MIN()
. kmem_alloc()
takes a size_t
, which is unsigned, converting the negative dynsize
value to a large positive value. When this gets rounded up to a page boundary, the value overflows, and the system panics:
panic[cpu3]/thread=fffffe0e1d978c00: vmem_xalloc(): size overflow fffffe0010298690 genunix:process_type+2670b5 () fffffe0010298710 genunix:vmem_alloc+1a0 () fffffe0010298750 genunix:kmem_alloc+19b () fffffe0010298930 elfexec:elfexec+4ef () fffffe0010298c00 genunix:gexec+677 () fffffe0010298e70 genunix:exec_common+5d3 () fffffe0010298ea0 genunix:exece+12 () fffffe0010298f00 unix:brand_sys_sysenter+2df ()
This was fixed in illumos-joyent as OS-7630 .
Related issues
Updated by Electric Monk about 3 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit a647f7a8445a398295ac6ca738ef7e8311daa466
commit a647f7a8445a398295ac6ca738ef7e8311daa466 Author: Cody Peter Mello <cody.mello@joyent.com> Date: 2019-03-06T16:18:09.000Z 10505 elfexec() should keep to unsigned types when processing PT_DYNAMIC Reviewed by: Richard Lowe <richlowe@richlowe.net> Reviewed by: Patrick Mooney <patrick.mooney@joyent.com> Reviewed by: Andy Stormont <astormont@racktopsystems.com> Reviewed by: Andy Fiddaman <andy@omniosce.org> Reviewed by: Gergő Mihály Doma <domag02@gmail.com> Approved by: Robert Mustacchi <rm@joyent.com>
Updated by Andy Fiddaman 6 months ago
- Related to Bug #14236: signed math leads getelfshdr astray added