Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b1f8c80d authored by Jeff Tinker's avatar Jeff Tinker
Browse files

Fix security vulnerability in CryptoHal 

CryptoHal was not checking that the memory heap set by setHeap
was the same one that was actually used for the decrypt call, allowing
the caller to spoof the decrypt call into accessing arbitrary memory.

bug:76221123
test: mediadrmserverpoc included in the bug & GTS media tests
Change-Id: I35214a1a6d0a4b864123e147d1a1adc2377bfbc5
parent 1ab07d1d

drm/libmediadrm/CryptoHal.cpp

+20 −4
Original line number Diff line number Diff line
@@ -253,7 +253,7 @@ int32_t CryptoHal::setHeapBase(const sp<IMemoryHeap>& heap) { 


    int32_t seqNum = mHeapSeqNum++;

    sp<HidlMemory> hidlMemory = fromHeap(heap);

    mHeapBases.add(seqNum, mNextBufferId);

    mHeapBases.add(seqNum, HeapBase(mNextBufferId, heap->getSize()));

    Return<void> hResult = mPlugin->setSharedBufferBase(*hidlMemory, mNextBufferId++);

    ALOGE_IF(!hResult.isOk(), "setSharedBufferBase(): remote call failed");

    return seqNum;
@@ -278,10 +278,26 @@ status_t CryptoHal::toSharedBuffer(const sp<IMemory>& memory, int32_t seqNum, :: 
        return UNEXPECTED_NULL;

    }



    // memory must be in the declared heap

    CHECK(mHeapBases.indexOfKey(seqNum) >= 0);

    // memory must be in one of the heaps that have been set

    if (mHeapBases.indexOfKey(seqNum) < 0) {

        return UNKNOWN_ERROR;

    }



    // heap must be the same size as the one that was set in setHeapBase

    if (mHeapBases.valueFor(seqNum).getSize() != heap->getSize()) {

        android_errorWriteLog(0x534e4554, "76221123");

        return UNKNOWN_ERROR;

     }



    // memory must be within the address space of the heap

    if (memory->pointer() != static_cast<uint8_t *>(heap->getBase()) + memory->offset()  ||

            heap->getSize() < memory->offset() + memory->size() ||

            SIZE_MAX - memory->offset() < memory->size()) {

        android_errorWriteLog(0x534e4554, "76221123");

        return UNKNOWN_ERROR;

    }



    buffer->bufferId = mHeapBases.valueFor(seqNum);

    buffer->bufferId = mHeapBases.valueFor(seqNum).getBufferId();

    buffer->offset = offset >= 0 ? offset : 0;

    buffer->size = size;

    return OK;

media/libmedia/include/media/CryptoHal.h

+14 −1
Original line number Diff line number Diff line
@@ -81,7 +81,20 @@ private: 
     */

    status_t mInitCheck;



    KeyedVector<int32_t, uint32_t> mHeapBases;

    struct HeapBase {

        HeapBase() : mBufferId(0), mSize(0) {}

        HeapBase(uint32_t bufferId, size_t size) :

            mBufferId(bufferId), mSize(size) {}



        uint32_t getBufferId() const {return mBufferId;}

        size_t getSize() const {return mSize;}



    private:

        uint32_t mBufferId;

        size_t mSize;

    };



    KeyedVector<int32_t, HeapBase> mHeapBases;

    uint32_t mNextBufferId;

    int32_t mHeapSeqNum;