Project

General

Profile

Bug #10505

elfexec() should keep to unsigned types when processing PT_DYNAMIC

Added by Cody Mello 20 days ago. Updated 18 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
kernel
Start date:
2019-03-04
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

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 .

History

#1

Updated by Electric Monk 18 days ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

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>

Also available in: Atom PDF