diff options
author | Andy Hung <hunga@google.com> | 2018-04-12 11:06:56 -0700 |
---|---|---|
committer | MSe <mse1969@posteo.de> | 2018-06-08 19:08:54 +0200 |
commit | 33b6a0d4e2ce158fee4241985cfca3b959ec99a7 (patch) | |
tree | 3d93cc3615be51655e8326df557e30735da4418d | |
parent | 947e4ba1ceb5df5313a9f4878f8ec0fd285f9b36 (diff) | |
download | frameworks_av-33b6a0d4e2ce158fee4241985cfca3b959ec99a7.tar.gz frameworks_av-33b6a0d4e2ce158fee4241985cfca3b959ec99a7.tar.bz2 frameworks_av-33b6a0d4e2ce158fee4241985cfca3b959ec99a7.zip |
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 498bdcc90bc470a79bf8943cbac64502f7c1c091)
CVE-2018-9378
-rw-r--r-- | include/media/IAudioPolicyService.h | 2 | ||||
-rw-r--r-- | media/libmedia/IAudioPolicyService.cpp | 65 |
2 files changed, 53 insertions, 14 deletions
diff --git a/include/media/IAudioPolicyService.h b/include/media/IAudioPolicyService.h index 3db966e281..9441861635 100644 --- a/include/media/IAudioPolicyService.h +++ b/include/media/IAudioPolicyService.h @@ -182,6 +182,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); }; // ---------------------------------------------------------------------------- diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp index e1bd5611a5..7a80bad7e9 100644 --- a/media/libmedia/IAudioPolicyService.cpp +++ b/media/libmedia/IAudioPolicyService.cpp @@ -863,7 +863,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)); } @@ -879,7 +879,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)); @@ -949,7 +949,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(); @@ -1046,8 +1046,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; @@ -1055,8 +1058,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(); int session = data.readInt32(); @@ -1115,7 +1121,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) { @@ -1134,7 +1140,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); @@ -1189,7 +1195,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 = {}; if (data.read(&handle, sizeof(audio_patch_handle_t)) != NO_ERROR) { @@ -1205,7 +1211,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); @@ -1244,8 +1250,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; @@ -1336,9 +1343,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 = {}; @@ -1361,6 +1369,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; @@ -1370,6 +1386,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 |