Protect against TOC/TOU bugs in soundtrigger
Test: Ran the repro program vendor/google_toolbox/user/ytai/SoundTriggerPoc
Test: Manual verification that soundtrigger still works for hotword and music.
Fixes: 136005905
Change-Id: Icdfe5a823ba02eb7bd8c41f0fa503c9922d8f348
diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 69e5f50..ccbeb77 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -40,6 +40,32 @@
#define HW_MODULE_PREFIX "primary"
namespace android {
+namespace {
+
+// Given an IMemory, returns a copy of its content along with its size.
+// Returns nullptr on failure or if input is nullptr.
+std::pair<std::unique_ptr<uint8_t[]>,
+ size_t> CopyToArray(const sp<IMemory>& mem) {
+ if (mem == nullptr) {
+ return std::make_pair(nullptr, 0);
+ }
+
+ const size_t size = mem->size();
+ if (size == 0) {
+ return std::make_pair(nullptr, 0);
+ }
+
+ std::unique_ptr<uint8_t[]> ar = std::make_unique<uint8_t[]>(size);
+ if (ar == nullptr) {
+ return std::make_pair(nullptr, 0);
+ }
+
+ memcpy(ar.get(), mem->unsecurePointer(), size);
+ return std::make_pair(std::move(ar), size);
+}
+
+}
+
SoundTriggerHwService::SoundTriggerHwService()
: BnSoundTriggerHwService(),
mNextUniqueId(1),
@@ -557,12 +583,13 @@
return NO_INIT;
}
- // TODO: Using unsecurePointer() has some associated security pitfalls
- // (see declaration for details).
- // Either document why it is safe in this case or address the
- // issue (e.g. by copying).
- struct sound_trigger_sound_model *sound_model =
- (struct sound_trigger_sound_model *)modelMemory->unsecurePointer();
+ auto immutableMemory = CopyToArray(modelMemory);
+ if (immutableMemory.first == nullptr) {
+ return NO_MEMORY;
+ }
+
+ struct sound_trigger_sound_model* sound_model =
+ (struct sound_trigger_sound_model*) immutableMemory.first.get();
size_t structSize;
if (sound_model->type == SOUND_MODEL_TYPE_KEYPHRASE) {
@@ -572,9 +599,10 @@
}
if (sound_model->data_offset < structSize ||
- sound_model->data_size > (UINT_MAX - sound_model->data_offset) ||
- modelMemory->size() < sound_model->data_offset ||
- sound_model->data_size > (modelMemory->size() - sound_model->data_offset)) {
+ sound_model->data_size > (UINT_MAX - sound_model->data_offset) ||
+ immutableMemory.second < sound_model->data_offset ||
+ sound_model->data_size >
+ (immutableMemory.second - sound_model->data_offset)) {
android_errorWriteLog(0x534e4554, "30148546");
ALOGE("loadSoundModel() data_size is too big");
return BAD_VALUE;
@@ -655,17 +683,19 @@
return NO_INIT;
}
- // TODO: Using unsecurePointer() has some associated security pitfalls
- // (see declaration for details).
- // Either document why it is safe in this case or address the
- // issue (e.g. by copying).
- struct sound_trigger_recognition_config *config =
- (struct sound_trigger_recognition_config *)dataMemory->unsecurePointer();
+ auto immutableMemory = CopyToArray(dataMemory);
+ if (immutableMemory.first == nullptr) {
+ return NO_MEMORY;
+ }
+
+ struct sound_trigger_recognition_config* config =
+ (struct sound_trigger_recognition_config*) immutableMemory.first.get();
if (config->data_offset < sizeof(struct sound_trigger_recognition_config) ||
- config->data_size > (UINT_MAX - config->data_offset) ||
- dataMemory->size() < config->data_offset ||
- config->data_size > (dataMemory->size() - config->data_offset)) {
+ config->data_size > (UINT_MAX - config->data_offset) ||
+ immutableMemory.second < config->data_offset ||
+ config->data_size >
+ (immutableMemory.second - config->data_offset)) {
ALOGE("startRecognition() data_size is too big");
return BAD_VALUE;
}