OSDN Git Service

aaudio: add validation to improve security
authorPhil Burk <philburk@google.com>
Fri, 26 May 2017 23:05:02 +0000 (16:05 -0700)
committerEric Laurent <elaurent@google.com>
Thu, 22 Jun 2017 20:58:42 +0000 (20:58 +0000)
Check for obviously bad values to prevent common exploits.

Bug: 38205290
Test: CTS test_aaudio.cpp
Change-Id: Ic650dbfbcbb9b87a89aae080ee72910ab2c9409b
Signed-off-by: Phil Burk <philburk@google.com>
media/libaaudio/src/core/AudioStream.cpp
media/libaaudio/src/core/AudioStreamBuilder.cpp
media/libaaudio/src/core/AudioStreamBuilder.h

index e1e3c55..5a05c0e 100644 (file)
@@ -38,6 +38,12 @@ AudioStream::AudioStream()
 
 aaudio_result_t AudioStream::open(const AudioStreamBuilder& builder)
 {
+    // Call here as well because the AAudioService will call this without calling build().
+    aaudio_result_t result = builder.validate();
+    if (result != AAUDIO_OK) {
+        return result;
+    }
+
     // Copy parameters from the Builder because the Builder may be deleted after this call.
     mSamplesPerFrame = builder.getSamplesPerFrame();
     mSampleRate = builder.getSampleRate();
@@ -62,38 +68,6 @@ aaudio_result_t AudioStream::open(const AudioStreamBuilder& builder)
     ALOGI("AudioStream::open() device = %d, perfMode = %d, callbackFrames = %d",
           mDeviceId, mPerformanceMode, mFramesPerDataCallback);
 
-    // Check for values that are ridiculously out of range to prevent math overflow exploits.
-    // The service will do a better check.
-    if (mSamplesPerFrame < 0 || mSamplesPerFrame > 128) {
-        ALOGE("AudioStream::open(): samplesPerFrame out of range = %d", mSamplesPerFrame);
-        return AAUDIO_ERROR_OUT_OF_RANGE;
-    }
-
-    switch(mFormat) {
-        case AAUDIO_FORMAT_UNSPECIFIED:
-        case AAUDIO_FORMAT_PCM_I16:
-        case AAUDIO_FORMAT_PCM_FLOAT:
-            break; // valid
-        default:
-            ALOGE("AudioStream::open(): audioFormat not valid = %d", mFormat);
-            return AAUDIO_ERROR_INVALID_FORMAT;
-            // break;
-    }
-
-    if (mSampleRate != AAUDIO_UNSPECIFIED && (mSampleRate < 8000 || mSampleRate > 1000000)) {
-        ALOGE("AudioStream::open(): mSampleRate out of range = %d", mSampleRate);
-        return AAUDIO_ERROR_INVALID_RATE;
-    }
-
-    switch(mPerformanceMode) {
-        case AAUDIO_PERFORMANCE_MODE_NONE:
-        case AAUDIO_PERFORMANCE_MODE_POWER_SAVING:
-        case AAUDIO_PERFORMANCE_MODE_LOW_LATENCY:
-            break;
-        default:
-            ALOGE("AudioStream::open(): illegal performanceMode %d", mPerformanceMode);
-            return AAUDIO_ERROR_ILLEGAL_ARGUMENT;
-    }
 
     return AAUDIO_OK;
 }
index 4262f27..8a481c0 100644 (file)
@@ -37,6 +37,19 @@ using namespace aaudio;
 #define AAUDIO_MMAP_POLICY_DEFAULT             AAUDIO_POLICY_NEVER
 #define AAUDIO_MMAP_EXCLUSIVE_POLICY_DEFAULT   AAUDIO_POLICY_NEVER
 
+// These values are for a pre-check before we ask the lower level service to open a stream.
+// So they are just outside the maximum conceivable range of value,
+// on the edge of being ridiculous.
+// TODO These defines should be moved to a central place in audio.
+#define SAMPLES_PER_FRAME_MIN        1
+// TODO Remove 8 channel limitation.
+#define SAMPLES_PER_FRAME_MAX        FCC_8
+#define SAMPLE_RATE_HZ_MIN           8000
+// HDMI supports up to 32 channels at 1536000 Hz.
+#define SAMPLE_RATE_HZ_MAX           1600000
+#define FRAMES_PER_DATA_CALLBACK_MIN 1
+#define FRAMES_PER_DATA_CALLBACK_MAX (1024 * 1024)
+
 /*
  * AudioStreamBuilder
  */
@@ -85,8 +98,17 @@ static aaudio_result_t builder_createStream(aaudio_direction_t direction,
 // Exact behavior is controlled by MMapPolicy.
 aaudio_result_t AudioStreamBuilder::build(AudioStream** streamPtr) {
     AudioStream *audioStream = nullptr;
+    if (streamPtr == nullptr) {
+        ALOGE("AudioStreamBuilder::build() streamPtr is null");
+        return AAUDIO_ERROR_NULL;
+    }
     *streamPtr = nullptr;
 
+    aaudio_result_t result = validate();
+    if (result != AAUDIO_OK) {
+        return result;
+    }
+
     // The API setting is the highest priority.
     aaudio_policy_t mmapPolicy = AAudio_getMMapPolicy();
     // If not specified then get from a system property.
@@ -116,8 +138,7 @@ aaudio_result_t AudioStreamBuilder::build(AudioStream** streamPtr) {
     bool allowMMap = mmapPolicy != AAUDIO_POLICY_NEVER;
     bool allowLegacy = mmapPolicy != AAUDIO_POLICY_ALWAYS;
 
-    aaudio_result_t result = builder_createStream(getDirection(), sharingMode,
-                                                  allowMMap, &audioStream);
+    result = builder_createStream(getDirection(), sharingMode, allowMMap, &audioStream);
     if (result == AAUDIO_OK) {
         // Open the stream using the parameters from the builder.
         result = audioStream->open(*this);
@@ -147,3 +168,83 @@ aaudio_result_t AudioStreamBuilder::build(AudioStream** streamPtr) {
 
     return result;
 }
+
+aaudio_result_t AudioStreamBuilder::validate() const {
+
+    // Check for values that are ridiculously out of range to prevent math overflow exploits.
+    // The service will do a better check.
+    if (mSamplesPerFrame != AAUDIO_UNSPECIFIED
+        && (mSamplesPerFrame < SAMPLES_PER_FRAME_MIN || mSamplesPerFrame > SAMPLES_PER_FRAME_MAX)) {
+        ALOGE("AudioStreamBuilder: channelCount out of range = %d", mSamplesPerFrame);
+        return AAUDIO_ERROR_OUT_OF_RANGE;
+    }
+
+    if (mDeviceId < 0) {
+        ALOGE("AudioStreamBuilder: deviceId out of range = %d", mDeviceId);
+        return AAUDIO_ERROR_OUT_OF_RANGE;
+    }
+
+    switch (mSharingMode) {
+        case AAUDIO_SHARING_MODE_EXCLUSIVE:
+        case AAUDIO_SHARING_MODE_SHARED:
+            break;
+        default:
+            ALOGE("AudioStreamBuilder: illegal sharingMode = %d", mSharingMode);
+            return AAUDIO_ERROR_ILLEGAL_ARGUMENT;
+            // break;
+    }
+
+    switch (mFormat) {
+        case AAUDIO_FORMAT_UNSPECIFIED:
+        case AAUDIO_FORMAT_PCM_I16:
+        case AAUDIO_FORMAT_PCM_FLOAT:
+            break; // valid
+        default:
+            ALOGE("AudioStreamBuilder: audioFormat not valid = %d", mFormat);
+            return AAUDIO_ERROR_INVALID_FORMAT;
+            // break;
+    }
+
+    switch (mDirection) {
+        case AAUDIO_DIRECTION_INPUT:
+        case AAUDIO_DIRECTION_OUTPUT:
+            break; // valid
+        default:
+            ALOGE("AudioStreamBuilder: direction not valid = %d", mDirection);
+            return AAUDIO_ERROR_ILLEGAL_ARGUMENT;
+            // break;
+    }
+
+    if (mSampleRate != AAUDIO_UNSPECIFIED
+        && (mSampleRate < SAMPLE_RATE_HZ_MIN || mSampleRate > SAMPLE_RATE_HZ_MAX)) {
+        ALOGE("AudioStreamBuilder: sampleRate out of range = %d", mSampleRate);
+        return AAUDIO_ERROR_INVALID_RATE;
+    }
+
+    if (mBufferCapacity < 0) {
+        ALOGE("AudioStreamBuilder: bufferCapacity out of range = %d", mBufferCapacity);
+        return AAUDIO_ERROR_OUT_OF_RANGE;
+    }
+
+    switch (mPerformanceMode) {
+        case AAUDIO_PERFORMANCE_MODE_NONE:
+        case AAUDIO_PERFORMANCE_MODE_POWER_SAVING:
+        case AAUDIO_PERFORMANCE_MODE_LOW_LATENCY:
+            break;
+        default:
+            ALOGE("AudioStreamBuilder: illegal performanceMode = %d", mPerformanceMode);
+            return AAUDIO_ERROR_ILLEGAL_ARGUMENT;
+            // break;
+    }
+
+    // Prevent ridiculous values from causing problems.
+    if (mFramesPerDataCallback != AAUDIO_UNSPECIFIED
+        && (mFramesPerDataCallback < FRAMES_PER_DATA_CALLBACK_MIN
+            || mFramesPerDataCallback > FRAMES_PER_DATA_CALLBACK_MAX)) {
+        ALOGE("AudioStreamBuilder: framesPerDataCallback out of range = %d",
+              mFramesPerDataCallback);
+        return AAUDIO_ERROR_OUT_OF_RANGE;
+    }
+
+    return AAUDIO_OK;
+}
\ No newline at end of file
index fd416c4..d757592 100644 (file)
@@ -165,6 +165,8 @@ public:
 
     aaudio_result_t build(AudioStream **streamPtr);
 
+    aaudio_result_t validate() const;
+
 private:
     int32_t                    mSamplesPerFrame = AAUDIO_UNSPECIFIED;
     int32_t                    mSampleRate = AAUDIO_UNSPECIFIED;