summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKevin Rocard <krocard@google.com>2017-11-13 11:15:27 -0800
committerIvan Kutepov <its.kutepov@gmail.com>2018-02-08 02:41:00 +0300
commitd43b790fe4e45c0a0ab598e169672af0904dc0d3 (patch)
tree895ee3976fd1c9d09b3ed1bbf3a34fcf7f3692cc
parent19d12edc1aad955ecd2e2b1bc786f1e7acb5fe0c (diff)
downloadframeworks_av-d43b790fe4e45c0a0ab598e169672af0904dc0d3.tar.gz
frameworks_av-d43b790fe4e45c0a0ab598e169672af0904dc0d3.tar.bz2
frameworks_av-d43b790fe4e45c0a0ab598e169672af0904dc0d3.zip
IAudioPolicyService: Add attribute tags sanitization
When audio_attributes_t was read from the binder parcel, the string tags field was copied without checking that it contained a '\0'. This could lead to read past the end when tags were used. This patch always adds a '\0' at the end of the buffer when deserializing. Bug: 68953950 Test: manual playback/record Test: send binder payload without \0 in tags attribute, check that only AUDIO_ATTRIBUTES_TAGS_MAX_SIZE - 1 char are printed. Change-Id: I285258cbf7cfaf26b191d1f31b3b1e2d724c4934 Merged-In: I285258cbf7cfaf26b191d1f31b3b1e2d724c4934 Signed-off-by: Kevin Rocard <krocard@google.com> (cherry picked from commit 39fdbd097a147b5c719dac9ad2759e6c44eb3a4e) CVE-2017-13232
-rw-r--r--include/media/IAudioPolicyService.h2
-rw-r--r--media/libmedia/IAudioPolicyService.cpp12
2 files changed, 14 insertions, 0 deletions
diff --git a/include/media/IAudioPolicyService.h b/include/media/IAudioPolicyService.h
index 1df91ee140..3db966e281 100644
--- a/include/media/IAudioPolicyService.h
+++ b/include/media/IAudioPolicyService.h
@@ -180,6 +180,8 @@ public:
const Parcel& data,
Parcel* reply,
uint32_t flags = 0);
+private:
+ void sanetizeAudioAttributes(audio_attributes_t* attr);
};
// ----------------------------------------------------------------------------
diff --git a/media/libmedia/IAudioPolicyService.cpp b/media/libmedia/IAudioPolicyService.cpp
index abae614645..e1bd5611a5 100644
--- a/media/libmedia/IAudioPolicyService.cpp
+++ b/media/libmedia/IAudioPolicyService.cpp
@@ -883,6 +883,7 @@ status_t BnAudioPolicyService::onTransact(
bool hasAttributes = data.readInt32() != 0;
if (hasAttributes) {
data.read(&attr, sizeof(audio_attributes_t));
+ sanetizeAudioAttributes(&attr);
}
audio_session_t session = (audio_session_t)data.readInt32();
audio_stream_type_t stream = AUDIO_STREAM_DEFAULT;
@@ -950,6 +951,7 @@ status_t BnAudioPolicyService::onTransact(
CHECK_INTERFACE(IAudioPolicyService, data, reply);
audio_attributes_t attr;
data.read(&attr, sizeof(audio_attributes_t));
+ sanetizeAudioAttributes(&attr);
audio_session_t session = (audio_session_t)data.readInt32();
uid_t uid = (uid_t)data.readInt32();
uint32_t samplingRate = data.readInt32();
@@ -1338,6 +1340,7 @@ status_t BnAudioPolicyService::onTransact(
data.read(&source, sizeof(struct audio_port_config));
audio_attributes_t attributes;
data.read(&attributes, sizeof(audio_attributes_t));
+ sanetizeAudioAttributes(&attributes);
audio_io_handle_t handle = {};
status_t status = startAudioSource(&source, &attributes, &handle);
reply->writeInt32(status);
@@ -1358,6 +1361,15 @@ status_t BnAudioPolicyService::onTransact(
}
}
+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';
+}
+
// ----------------------------------------------------------------------------
} // namespace android