Description

There is an OOB read vulnerability exists in IOGraphics due to wrong create/release logic.

Environment

OS: macOS 10.13.5
Module: IOGraphics.kext

Analysis

In IDState class, the CreateID method can create IDs in different ways according to parameter ‘options’:

    IOReturn createID(const bool taskOwned, const IOOptionBits options,
                      const IOAccelID requestedID, IOAccelID *idOutP)
    {
        const uint32_t taskOwnedMask = (taskOwned) ? kTaskOwned : 0;
        IOReturn ret;
        if (kIOAccelSpecificID & options)
            ret = reserveRequested(taskOwnedMask, requestedID, idOutP);
        else
            ret = allocID(taskOwnedMask, idOutP);
        return ret;
    }

It can either call reserveRequested or allocID. The 2 methods store id into different queues, the fSpecifiedIDsData and fRetainIDsData. But when release the previous created id, the logic of choosing queue doesn’t match the one in CreateID. The logic exists in method locateID:

    IDDataDecode locateID(IOAccelID id)
    {
        IDDataDecode ret = {
            .fIDsData = fRetainIDsData,
            .fArrayInd = id - kAllocatedIDBase,
        };
        if (static_cast<unsigned>(id) < kAllocatedIDBase) {
            ret.fIDsData = fSpecifiedIDsData;
            ret.fArrayInd = int2zz(id);
        }
        ret.fUP = getID(ret.fIDsData, ret.fArrayInd);
        return ret;
    }

It chooses queue according to the id value , not the options any more. So we can go to the wrong queue with a very big id value, e.g. -2. Finally the function getID uses the very big id value as its index:

// Range checked lookup of a uint32_t within a data
inline uint32_t *getID(OSData *data, int index)
{
    const uint32_t *uP = static_cast<const uint32_t*>(
            data->getBytesNoCopy(index * sizeof(*uP), sizeof(*uP)));
    return const_cast<uint32_t*>(uP);
}

Here we get out of boundary read in function getBytesNoCopy.