commit 2a45115039163601f30e9b87b9ea8f1674713dce
author Eric Laurent <elaurent@google.com> Wed Oct 07 13:48:21 2020 -0700
committer Eric Laurent <elaurent@google.com> Tue Dec 08 21:52:16 2020 +0100
tree 832db337f9a0ae13d92e58bdafbebb2cbbbe1628
parent fff9c8aa87a7c03e063cf6adeb4c63b895b6d145

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

media/libaudioclient/IAudioFlinger.cpp

diff --git a/media/libaudioclient/IAudioFlinger.cpp b/media/libaudioclient/IAudioFlinger.cpp
index 16d2232..d3a037c 100644
--- a/media/libaudioclient/IAudioFlinger.cpp
+++ b/media/libaudioclient/IAudioFlinger.cpp
@@ -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 @@
         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_t status = getAudioPort(&port);
+            status = AudioSanitizer::sanitizeAudioPort(&port);
+            if (status == NO_ERROR) {
+                status = getAudioPort(&port);
+            }
             reply->writeInt32(status);
             if (status == NO_ERROR) {
                 reply->write(&port, sizeof(struct audio_port));
@@ -1496,12 +1502,20 @@
         case CREATE_AUDIO_PATCH: {
             CHECK_INTERFACE(IAudioFlinger, data, reply);
             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) {
-                ALOGE("b/23905951");
+            status_t status = data.read(&patch, sizeof(struct audio_patch));
+            if (status != NO_ERROR) {
+                return status;
             }
-            status_t status = createAudioPatch(&patch, &handle);
+            audio_patch_handle_t handle = AUDIO_PATCH_HANDLE_NONE;
+            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);
+            }
             reply->writeInt32(status);
             if (status == NO_ERROR) {
                 reply->write(&handle, sizeof(audio_patch_handle_t));
@@ -1546,8 +1560,14 @@
         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;