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

Commit 92320ba0 authored by Andy Hung's avatar Andy Hung Committed by Rohit Yengisetty
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)
parent e8298a73

media/libaudioclient/IAudioPolicyService.cpp

+51 −14
Original line number Diff line number Diff line
@@ -940,7 +940,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));

            }
@@ -956,7 +956,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));
@@ -1024,7 +1024,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_io_handle_t input = (audio_io_handle_t)data.readInt32();
@@ -1125,8 +1125,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;
@@ -1134,8 +1137,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();
@@ -1194,7 +1200,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) {
@@ -1213,7 +1219,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);
@@ -1268,7 +1274,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) {
@@ -1284,7 +1290,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);
@@ -1323,8 +1329,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;
@@ -1398,9 +1405,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_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;
@@ -1453,6 +1461,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;
@@ -1462,6 +1478,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

media/libaudioclient/include/media/IAudioPolicyService.h

+2 −0
Original line number Diff line number Diff line
@@ -185,6 +185,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);

};



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