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

Commit 5e6081c0 authored by Andy Hung's avatar Andy Hung Committed by Michael W
Browse files

Sanitize effect descriptors for AudioPolicyService binder calls. 

Zero initialize structs before parcel read, if status is not checked.
Sanitize parcel read audio_port_config.

Test: Audio CTS, See bug for POC
Bug: 73126106
Merged-in: Iece43eb463385927e6babcf93654eea8aaebc29c
Change-Id: Iece43eb463385927e6babcf93654eea8aaebc29c
(cherry picked from commit 498bdcc9)
CVE-2018-9378
parent 2ac21004

include/media/IAudioPolicyService.h

+2 −0
Original line number Diff line number Diff line
@@ -186,6 +186,8 @@ public: 
                                    uint32_t flags = 0);

private:

    void sanetizeAudioAttributes(audio_attributes_t* attr);

    status_t sanitizeEffectDescriptor(effect_descriptor_t* desc);

    status_t sanitizeAudioPortConfig(struct audio_port_config* config);

};



// ----------------------------------------------------------------------------

media/libmedia/IAudioPolicyService.cpp

+51 −14
Original line number Diff line number Diff line
@@ -897,7 +897,7 @@ status_t BnAudioPolicyService::onTransact( 
            audio_output_flags_t flags =

                    static_cast <audio_output_flags_t>(data.readInt32());

            bool hasOffloadInfo = data.readInt32() != 0;

            audio_offload_info_t offloadInfo;

            audio_offload_info_t offloadInfo = {};

            if (hasOffloadInfo) {

                data.read(&offloadInfo, sizeof(audio_offload_info_t));

            }
@@ -913,7 +913,7 @@ status_t BnAudioPolicyService::onTransact( 


        case GET_OUTPUT_FOR_ATTR: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            audio_attributes_t attr;

            audio_attributes_t attr = {};

            bool hasAttributes = data.readInt32() != 0;

            if (hasAttributes) {

                data.read(&attr, sizeof(audio_attributes_t));
@@ -983,7 +983,7 @@ status_t BnAudioPolicyService::onTransact( 


        case GET_INPUT_FOR_ATTR: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            audio_attributes_t attr;

            audio_attributes_t attr = {};

            data.read(&attr, sizeof(audio_attributes_t));

            sanetizeAudioAttributes(&attr);

            audio_session_t session = (audio_session_t)data.readInt32();
@@ -1081,8 +1081,11 @@ status_t BnAudioPolicyService::onTransact( 


        case GET_OUTPUT_FOR_EFFECT: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            effect_descriptor_t desc;

            data.read(&desc, sizeof(effect_descriptor_t));

            effect_descriptor_t desc = {};

            if (data.read(&desc, sizeof(desc)) != NO_ERROR) {

                android_errorWriteLog(0x534e4554, "73126106");

            }

            (void)sanitizeEffectDescriptor(&desc);

            audio_io_handle_t output = getOutputForEffect(&desc);

            reply->writeInt32(static_cast <int>(output));

            return NO_ERROR;
@@ -1090,8 +1093,11 @@ status_t BnAudioPolicyService::onTransact( 


        case REGISTER_EFFECT: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            effect_descriptor_t desc;

            data.read(&desc, sizeof(effect_descriptor_t));

            effect_descriptor_t desc = {};

            if (data.read(&desc, sizeof(desc)) != NO_ERROR) {

                android_errorWriteLog(0x534e4554, "73126106");

            }

            (void)sanitizeEffectDescriptor(&desc);

            audio_io_handle_t io = data.readInt32();

            uint32_t strategy = data.readInt32();

            audio_session_t session = (audio_session_t) data.readInt32();
@@ -1150,7 +1156,7 @@ status_t BnAudioPolicyService::onTransact( 
                count = AudioEffect::kMaxPreProcessing;

            }

            uint32_t retCount = count;

            effect_descriptor_t *descriptors = new effect_descriptor_t[count];

            effect_descriptor_t *descriptors = new effect_descriptor_t[count]{};

            status_t status = queryDefaultPreProcessing(audioSession, descriptors, &retCount);

            reply->writeInt32(status);

            if (status != NO_ERROR && status != NO_MEMORY) {
@@ -1169,7 +1175,7 @@ status_t BnAudioPolicyService::onTransact( 


        case IS_OFFLOAD_SUPPORTED: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            audio_offload_info_t info;

            audio_offload_info_t info = {};

            data.read(&info, sizeof(audio_offload_info_t));

            bool isSupported = isOffloadSupported(info);

            reply->writeInt32(isSupported);
@@ -1224,7 +1230,7 @@ status_t BnAudioPolicyService::onTransact( 


        case CREATE_AUDIO_PATCH: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_patch patch;

            struct audio_patch patch = {};

            data.read(&patch, sizeof(struct audio_patch));

            audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;

            if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) {
@@ -1240,7 +1246,7 @@ status_t BnAudioPolicyService::onTransact( 


        case RELEASE_AUDIO_PATCH: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            audio_patch_handle_t handle;

            audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;

            data.read(&handle, sizeof(audio_patch_handle_t));

            status_t status = releaseAudioPatch(handle);

            reply->writeInt32(status);
@@ -1279,8 +1285,9 @@ status_t BnAudioPolicyService::onTransact( 


        case SET_AUDIO_PORT_CONFIG: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_port_config config;

            struct audio_port_config config = {};

            data.read(&config, sizeof(struct audio_port_config));

            (void)sanitizeAudioPortConfig(&config);

            status_t status = setAudioPortConfig(&config);

            reply->writeInt32(status);

            return NO_ERROR;
@@ -1371,9 +1378,10 @@ status_t BnAudioPolicyService::onTransact( 


        case START_AUDIO_SOURCE: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_port_config source;

            struct audio_port_config source = {};

            data.read(&source, sizeof(struct audio_port_config));

            audio_attributes_t attributes;

            (void)sanitizeAudioPortConfig(&source);

            audio_attributes_t attributes = {};

            data.read(&attributes, sizeof(audio_attributes_t));

            sanetizeAudioAttributes(&attributes);

            audio_io_handle_t handle = {};
@@ -1415,6 +1423,14 @@ status_t BnAudioPolicyService::onTransact( 
    }

}



/** returns true if string overflow was prevented by zero termination */

template <size_t size>

static bool preventStringOverflow(char (&s)[size]) {

    if (strnlen(s, size) < size) return false;

    s[size - 1] = '\0';

    return true;

}



void BnAudioPolicyService::sanetizeAudioAttributes(audio_attributes_t* attr)

{

    const size_t tagsMaxSize = AUDIO_ATTRIBUTES_TAGS_MAX_SIZE;
@@ -1424,6 +1440,27 @@ void BnAudioPolicyService::sanetizeAudioAttributes(audio_attributes_t* attr) 
    attr->tags[tagsMaxSize - 1] = '\0';

}



/** returns BAD_VALUE if sanitization was required. */

status_t BnAudioPolicyService::sanitizeEffectDescriptor(effect_descriptor_t* desc)

{

    if (preventStringOverflow(desc->name)

        | /* always */ preventStringOverflow(desc->implementor)) {

        android_errorWriteLog(0x534e4554, "73126106"); // SafetyNet logging

        return BAD_VALUE;

    }

    return NO_ERROR;

}



/** returns BAD_VALUE if sanitization was required. */

status_t BnAudioPolicyService::sanitizeAudioPortConfig(struct audio_port_config* config)

{

    if (config->type == AUDIO_PORT_TYPE_DEVICE &&

        preventStringOverflow(config->ext.device.address)) {

        return BAD_VALUE;

    }

    return NO_ERROR;

}



// ----------------------------------------------------------------------------



} // namespace android