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

Commit 2a451150 authored by Eric Laurent's avatar Eric Laurent
Browse files

IAudioPolicyService, IAudioFlinger: check C structs on server side 

Add more checks on C structs received on server side to avoid string
overflows in particular.
Move sanitization helpers into a specific class in media helper library.

Bug: 169572641
Test: audio smoke tests
Change-Id: I89cb840230c85dcdddd5b128f626bb4fe303832a
Merged-In: I89cb840230c85dcdddd5b128f626bb4fe303832a
parent fff9c8aa

media/libaudioclient/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ cc_library_shared { 


        "AudioEffect.cpp",

        "AudioRecord.cpp",

        "AudioSanitizer.cpp",

        "AudioSystem.cpp",

        "AudioTrack.cpp",

        "AudioTrackShared.cpp",

media/libaudioclient/AudioSanitizer.cpp

0 → 100644
+116 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#include <media/AudioSanitizer.h>



namespace android {



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

template <size_t size>

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

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

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

    return true;

}



status_t safetyNetLog(status_t status, const char *bugNumber) {

    if (status != NO_ERROR && bugNumber != nullptr) {

        android_errorWriteLog(0x534e4554, bugNumber); // SafetyNet logging

    }

    return status;

}



status_t AudioSanitizer::sanitizeAudioAttributes(

        audio_attributes_t *attr, const char *bugNumber)

{

    status_t status = NO_ERROR;

    const size_t tagsMaxSize = AUDIO_ATTRIBUTES_TAGS_MAX_SIZE;

    if (strnlen(attr->tags, tagsMaxSize) >= tagsMaxSize) {

        status = BAD_VALUE;

    }

    attr->tags[tagsMaxSize - 1] = '\0';

    return safetyNetLog(status, bugNumber);

}



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

status_t AudioSanitizer::sanitizeEffectDescriptor(

        effect_descriptor_t *desc, const char *bugNumber)

{

    status_t status = NO_ERROR;

    if (preventStringOverflow(desc->name)

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

        status = BAD_VALUE;

    }

    return safetyNetLog(status, bugNumber);

}



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

status_t AudioSanitizer::sanitizeAudioPortConfig(

        struct audio_port_config *config, const char *bugNumber)

{

    status_t status = NO_ERROR;

    if (config->type == AUDIO_PORT_TYPE_DEVICE &&

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

        status = BAD_VALUE;

    }

    return safetyNetLog(status, bugNumber);

}



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

status_t AudioSanitizer::sanitizeAudioPort(

        struct audio_port *port, const char *bugNumber)

{

    status_t status = NO_ERROR;

    if (preventStringOverflow(port->name)) {

        status = BAD_VALUE;

    }

    if (sanitizeAudioPortConfig(&port->active_config) != NO_ERROR) {

        status = BAD_VALUE;

    }

    if (port->type == AUDIO_PORT_TYPE_DEVICE &&

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

        status = BAD_VALUE;

    }

    return safetyNetLog(status, bugNumber);

}



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

status_t AudioSanitizer::sanitizeAudioPatch(

        struct audio_patch *patch, const char *bugNumber)

{

    status_t status = NO_ERROR;

    if (patch->num_sources > AUDIO_PATCH_PORTS_MAX) {

        patch->num_sources = AUDIO_PATCH_PORTS_MAX;

        status = BAD_VALUE;

    }

    if (patch->num_sinks > AUDIO_PATCH_PORTS_MAX) {

        patch->num_sinks = AUDIO_PATCH_PORTS_MAX;

        status = BAD_VALUE;

    }

    for (size_t i = 0; i < patch->num_sources; i++) {

        if (sanitizeAudioPortConfig(&patch->sources[i]) != NO_ERROR) {

            status = BAD_VALUE;

        }

    }

    for (size_t i = 0; i < patch->num_sinks; i++) {

        if (sanitizeAudioPortConfig(&patch->sinks[i]) != NO_ERROR) {

            status = BAD_VALUE;

        }

    }

    return safetyNetLog(status, bugNumber);

}



}; // namespace android

media/libaudioclient/IAudioFlinger.cpp

+27 −7
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 


#include <binder/IPCThreadState.h>

#include <binder/Parcel.h>

#include <media/AudioSanitizer.h>

#include <mediautils/ServiceUtilities.h>

#include <mediautils/TimeCheck.h>

#include "IAudioFlinger.h"
@@ -1483,10 +1484,15 @@ status_t BnAudioFlinger::onTransact( 
        case GET_AUDIO_PORT: {

            CHECK_INTERFACE(IAudioFlinger, data, reply);

            struct audio_port port = {};

            if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) {

            status_t status = data.read(&port, sizeof(struct audio_port));

            if (status != NO_ERROR) {

                ALOGE("b/23905951");

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPort(&port);

            if (status == NO_ERROR) {

                status = getAudioPort(&port);

            }

            status_t status = getAudioPort(&port);

            reply->writeInt32(status);

            if (status == NO_ERROR) {

                reply->write(&port, sizeof(struct audio_port));
@@ -1496,12 +1502,20 @@ status_t BnAudioFlinger::onTransact( 
        case CREATE_AUDIO_PATCH: {

            CHECK_INTERFACE(IAudioFlinger, data, reply);

            struct audio_patch patch;

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

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

            if (status != NO_ERROR) {

                return status;

            }

            audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;

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

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

            if (status != NO_ERROR) {

                ALOGE("b/23905951");

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPatch(&patch);

            if (status == NO_ERROR) {

                status = createAudioPatch(&patch, &handle);

            }

            status_t status = createAudioPatch(&patch, &handle);

            reply->writeInt32(status);

            if (status == NO_ERROR) {

                reply->write(&handle, sizeof(audio_patch_handle_t));
@@ -1546,8 +1560,14 @@ status_t BnAudioFlinger::onTransact( 
        case SET_AUDIO_PORT_CONFIG: {

            CHECK_INTERFACE(IAudioFlinger, data, reply);

            struct audio_port_config config;

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

            status_t status = setAudioPortConfig(&config);

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

            if (status != NO_ERROR) {

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPortConfig(&config);

            if (status == NO_ERROR) {

                status = setAudioPortConfig(&config);

            }

            reply->writeInt32(status);

            return NO_ERROR;

        } break;

media/libaudioclient/IAudioPolicyService.cpp

+105 −81
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ 
#include <binder/IPCThreadState.h>

#include <binder/Parcel.h>

#include <media/AudioEffect.h>

#include <media/AudioSanitizer.h>

#include <media/IAudioPolicyService.h>

#include <mediautils/ServiceUtilities.h>

#include <mediautils/TimeCheck.h>
@@ -1685,7 +1686,6 @@ status_t BnAudioPolicyService::onTransact( 
            if (status != NO_ERROR) {

                return status;

            }

            sanetizeAudioAttributes(&attr);

            audio_session_t session = (audio_session_t)data.readInt32();

            audio_stream_type_t stream = AUDIO_STREAM_DEFAULT;

            bool hasStream = data.readInt32() != 0;
@@ -1703,10 +1703,14 @@ status_t BnAudioPolicyService::onTransact( 
            audio_port_handle_t portId = (audio_port_handle_t)data.readInt32();

            audio_io_handle_t output = 0;

            std::vector<audio_io_handle_t> secondaryOutputs;



            status = AudioSanitizer::sanitizeAudioAttributes(&attr, "68953950");

            if (status == NO_ERROR) {

                status = getOutputForAttr(&attr,

                                          &output, session, &stream, pid, uid,

                                          &config,

                                          flags, &selectedDeviceId, &portId, &secondaryOutputs);

            }

            reply->writeInt32(status);

            status = reply->write(&attr, sizeof(audio_attributes_t));

            if (status != NO_ERROR) {
@@ -1745,8 +1749,11 @@ status_t BnAudioPolicyService::onTransact( 
        case GET_INPUT_FOR_ATTR: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            audio_attributes_t attr = {};

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

            sanetizeAudioAttributes(&attr);

            status_t status = data.read(&attr, sizeof(audio_attributes_t));

            if (status != NO_ERROR) {

                return status;

            }



            audio_io_handle_t input = (audio_io_handle_t)data.readInt32();

            audio_unique_id_t riid = (audio_unique_id_t)data.readInt32();

            audio_session_t session = (audio_session_t)data.readInt32();
@@ -1759,9 +1766,13 @@ status_t BnAudioPolicyService::onTransact( 
            audio_input_flags_t flags = (audio_input_flags_t) data.readInt32();

            audio_port_handle_t selectedDeviceId = (audio_port_handle_t) data.readInt32();

            audio_port_handle_t portId = (audio_port_handle_t)data.readInt32();

            status_t status = getInputForAttr(&attr, &input, riid, session, pid, uid,



            status = AudioSanitizer::sanitizeAudioAttributes(&attr, "68953950");

            if (status == NO_ERROR) {

                status = getInputForAttr(&attr, &input, riid, session, pid, uid,

                                         opPackageName, &config,

                                         flags, &selectedDeviceId, &portId);

            }

            reply->writeInt32(status);

            if (status == NO_ERROR) {

                reply->writeInt32(input);
@@ -1842,11 +1853,15 @@ status_t BnAudioPolicyService::onTransact( 
            if (status != NO_ERROR) {

                return status;

            }



            int index = data.readInt32();

            audio_devices_t device = static_cast <audio_devices_t>(data.readInt32());



            reply->writeInt32(static_cast <uint32_t>(setVolumeIndexForAttributes(attributes,

                                                                                 index, device)));

            status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");

            if (status == NO_ERROR) {

                status = setVolumeIndexForAttributes(attributes, index, device);

            }

            reply->writeInt32(static_cast <int32_t>(status));

            return NO_ERROR;

        } break;
@@ -1860,8 +1875,11 @@ status_t BnAudioPolicyService::onTransact( 
            audio_devices_t device = static_cast <audio_devices_t>(data.readInt32());



            int index = 0;

            status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");

            if (status == NO_ERROR) {

                status = getVolumeIndexForAttributes(attributes, index, device);

            reply->writeInt32(static_cast <uint32_t>(status));

            }

            reply->writeInt32(static_cast <int32_t>(status));

            if (status == NO_ERROR) {

                reply->writeInt32(index);

            }
@@ -1877,8 +1895,11 @@ status_t BnAudioPolicyService::onTransact( 
            }



            int index = 0;

            status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");

            if (status == NO_ERROR) {

                status = getMinVolumeIndexForAttributes(attributes, index);

            reply->writeInt32(static_cast <uint32_t>(status));

            }

            reply->writeInt32(static_cast <int32_t>(status));

            if (status == NO_ERROR) {

                reply->writeInt32(index);

            }
@@ -1894,8 +1915,11 @@ status_t BnAudioPolicyService::onTransact( 
            }



            int index = 0;

            status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");

            if (status == NO_ERROR) {

                status = getMaxVolumeIndexForAttributes(attributes, index);

            reply->writeInt32(static_cast <uint32_t>(status));

            }

            reply->writeInt32(static_cast <int32_t>(status));

            if (status == NO_ERROR) {

                reply->writeInt32(index);

            }
@@ -1913,31 +1937,37 @@ status_t BnAudioPolicyService::onTransact( 
        case GET_OUTPUT_FOR_EFFECT: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            effect_descriptor_t desc = {};

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

            status_t status = data.read(&desc, sizeof(desc));

            if (status != NO_ERROR) {

                android_errorWriteLog(0x534e4554, "73126106");

                return status;

            }

            (void)sanitizeEffectDescriptor(&desc);

            audio_io_handle_t output = getOutputForEffect(&desc);

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

            audio_io_handle_t output = AUDIO_IO_HANDLE_NONE;

            status = AudioSanitizer::sanitizeEffectDescriptor(&desc, "73126106");

            if (status == NO_ERROR) {

                output = getOutputForEffect(&desc);

            }

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

            return NO_ERROR;

        } break;



        case REGISTER_EFFECT: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            effect_descriptor_t desc = {};

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

            status_t status = data.read(&desc, sizeof(desc));

            if (status != NO_ERROR) {

                android_errorWriteLog(0x534e4554, "73126106");

                return status;

            }

            (void)sanitizeEffectDescriptor(&desc);

            audio_io_handle_t io = data.readInt32();

            uint32_t strategy = data.readInt32();

            audio_session_t session = (audio_session_t) data.readInt32();

            int id = data.readInt32();

            reply->writeInt32(static_cast <int32_t>(registerEffect(&desc,

                                                                   io,

                                                                   strategy,

                                                                   session,

                                                                   id)));

            status = AudioSanitizer::sanitizeEffectDescriptor(&desc, "73126106");

            if (status == NO_ERROR) {

                status = registerEffect(&desc, io, strategy, session, id);

            }

            reply->writeInt32(static_cast <int32_t>(status));

            return NO_ERROR;

        } break;
@@ -2046,7 +2076,11 @@ status_t BnAudioPolicyService::onTransact( 
            if (status != NO_ERROR) return status;

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

            if (status != NO_ERROR) return status;

            reply->writeInt32(isDirectOutputSupported(config, attributes));

            status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");

            if (status == NO_ERROR) {

                status = isDirectOutputSupported(config, attributes);

            }

            reply->writeInt32(static_cast <int32_t>(status));

            return NO_ERROR;

        }
@@ -2085,10 +2119,15 @@ status_t BnAudioPolicyService::onTransact( 
        case GET_AUDIO_PORT: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_port port = {};

            if (data.read(&port, sizeof(struct audio_port)) != NO_ERROR) {

            status_t status = data.read(&port, sizeof(struct audio_port));

            if (status != NO_ERROR) {

                ALOGE("b/23912202");

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPort(&port);

            if (status == NO_ERROR) {

                status = getAudioPort(&port);

            }

            status_t status = getAudioPort(&port);

            reply->writeInt32(status);

            if (status == NO_ERROR) {

                reply->write(&port, sizeof(struct audio_port));
@@ -2099,12 +2138,20 @@ status_t BnAudioPolicyService::onTransact( 
        case CREATE_AUDIO_PATCH: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_patch patch = {};

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

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

            if (status != NO_ERROR) {

                return status;

            }

            audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;

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

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

            if (status != NO_ERROR) {

                ALOGE("b/23912202");

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPatch(&patch);

            if (status == NO_ERROR) {

                status = createAudioPatch(&patch, &handle);

            }

            status_t status = createAudioPatch(&patch, &handle);

            reply->writeInt32(status);

            if (status == NO_ERROR) {

                reply->write(&handle, sizeof(audio_patch_handle_t));
@@ -2154,9 +2201,12 @@ status_t BnAudioPolicyService::onTransact( 
        case SET_AUDIO_PORT_CONFIG: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_port_config config = {};

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

            (void)sanitizeAudioPortConfig(&config);

            status_t status = setAudioPortConfig(&config);

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

            if (status != NO_ERROR) {

                return status;

            }

            (void)AudioSanitizer::sanitizeAudioPortConfig(&config);

            status = setAudioPortConfig(&config);

            reply->writeInt32(status);

            return NO_ERROR;

        }
@@ -2232,13 +2282,25 @@ status_t BnAudioPolicyService::onTransact( 
        case START_AUDIO_SOURCE: {

            CHECK_INTERFACE(IAudioPolicyService, data, reply);

            struct audio_port_config source = {};

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

            (void)sanitizeAudioPortConfig(&source);

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

            if (status != NO_ERROR) {

                return status;

            }

            audio_attributes_t attributes = {};

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

            sanetizeAudioAttributes(&attributes);

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

            if (status != NO_ERROR) {

                return status;

            }

            status = AudioSanitizer::sanitizeAudioPortConfig(&source);

            if (status == NO_ERROR) {

                // OK to not always sanitize attributes as startAudioSource() is not called if

                // the port config is invalid.

                status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "68953950");

            }

            audio_port_handle_t portId = AUDIO_PORT_HANDLE_NONE;

            status_t status = startAudioSource(&source, &attributes, &portId);

            if (status == NO_ERROR) {

                status = startAudioSource(&source, &attributes, &portId);

            }

            reply->writeInt32(status);

            reply->writeInt32(portId);

            return NO_ERROR;
@@ -2762,44 +2824,6 @@ 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;

    if (strnlen(attr->tags, tagsMaxSize) >= tagsMaxSize) {

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

    }

    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/AudioSanitizer.h

0 → 100644
+47 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#ifndef ANDROID_AUDIO_SANITIZER_H_

#define ANDROID_AUDIO_SANITIZER_H_



#include <system/audio.h>

#include <system/audio_effect.h>

#include <utils/Errors.h>

#include <utils/Log.h>



namespace android {



class AudioSanitizer {

public:

    static status_t sanitizeAudioAttributes(

            audio_attributes_t *attr, const char *bugNumber = nullptr);



    static status_t sanitizeEffectDescriptor(

            effect_descriptor_t *desc, const char *bugNumber = nullptr);



    static status_t sanitizeAudioPortConfig(

            struct audio_port_config *config, const char *bugNumber = nullptr);



    static status_t sanitizeAudioPort(

            struct audio_port *port, const char *bugNumber = nullptr);



    static status_t sanitizeAudioPatch(

            struct audio_patch *patch, const char *bugNumber = nullptr);

};



}; // namespace android



#endif  /*ANDROID_AUDIO_SANITIZER_H_*/