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

Commit ea51ad6e authored by Ytai Ben-Tsvi's avatar Ytai Ben-Tsvi
Browse files

Protect against TOC/TOU bugs in soundtrigger 

Test: Ran the repro program vendor/google_toolbox/user/ytai/SoundTriggerPoc
Test: Manual verification that soundtrigger still works for hotword and music.
Fixes: 136005905

Change-Id: Icdfe5a823ba02eb7bd8c41f0fa503c9922d8f348
parent 071c3c87

services/soundtrigger/SoundTriggerHwService.cpp

+48 −18
Original line number Diff line number Diff line
@@ -40,6 +40,32 @@ 
#define HW_MODULE_PREFIX "primary"

namespace android {



namespace {



// Given an IMemory, returns a copy of its content along with its size.

// Returns nullptr on failure or if input is nullptr.

std::pair<std::unique_ptr<uint8_t[]>,

          size_t> CopyToArray(const sp<IMemory>& mem) {

    if (mem == nullptr) {

        return std::make_pair(nullptr, 0);

    }



    const size_t size = mem->size();

    if (size == 0) {

        return std::make_pair(nullptr, 0);

    }



    std::unique_ptr<uint8_t[]> ar = std::make_unique<uint8_t[]>(size);

    if (ar == nullptr) {

        return std::make_pair(nullptr, 0);

    }



    memcpy(ar.get(), mem->unsecurePointer(), size);

    return std::make_pair(std::move(ar), size);

}



}



SoundTriggerHwService::SoundTriggerHwService()

    : BnSoundTriggerHwService(),

      mNextUniqueId(1),
@@ -557,12 +583,13 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp<IMemory>& modelM 
        return NO_INIT;

    }



    // TODO: Using unsecurePointer() has some associated security pitfalls

    //       (see declaration for details).

    //       Either document why it is safe in this case or address the

    //       issue (e.g. by copying).

    auto immutableMemory = CopyToArray(modelMemory);

    if (immutableMemory.first == nullptr) {

        return NO_MEMORY;

    }



    struct sound_trigger_sound_model* sound_model =

            (struct sound_trigger_sound_model *)modelMemory->unsecurePointer();

        (struct sound_trigger_sound_model*) immutableMemory.first.get();



    size_t structSize;

    if (sound_model->type == SOUND_MODEL_TYPE_KEYPHRASE) {
@@ -573,8 +600,9 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp<IMemory>& modelM 


    if (sound_model->data_offset < structSize ||

        sound_model->data_size > (UINT_MAX - sound_model->data_offset) ||

           modelMemory->size() < sound_model->data_offset ||

           sound_model->data_size > (modelMemory->size() - sound_model->data_offset)) {

        immutableMemory.second < sound_model->data_offset ||

            sound_model->data_size >

            (immutableMemory.second - sound_model->data_offset)) {

        android_errorWriteLog(0x534e4554, "30148546");

        ALOGE("loadSoundModel() data_size is too big");

        return BAD_VALUE;
@@ -655,17 +683,19 @@ status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t ha 
        return NO_INIT;

    }



    // TODO: Using unsecurePointer() has some associated security pitfalls

    //       (see declaration for details).

    //       Either document why it is safe in this case or address the

    //       issue (e.g. by copying).

    auto immutableMemory = CopyToArray(dataMemory);

    if (immutableMemory.first == nullptr) {

        return NO_MEMORY;

    }



    struct sound_trigger_recognition_config* config =

            (struct sound_trigger_recognition_config *)dataMemory->unsecurePointer();

        (struct sound_trigger_recognition_config*) immutableMemory.first.get();



    if (config->data_offset < sizeof(struct sound_trigger_recognition_config) ||

        config->data_size > (UINT_MAX - config->data_offset) ||

            dataMemory->size() < config->data_offset ||

            config->data_size > (dataMemory->size() - config->data_offset)) {

        immutableMemory.second < config->data_offset ||

            config->data_size >

            (immutableMemory.second - config->data_offset)) {

        ALOGE("startRecognition() data_size is too big");

        return BAD_VALUE;

    }