From 75fc175a08c1a8e86d4649c19fd3136121518b96 Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Mon, 22 Aug 2022 15:58:51 -0500 Subject: [PATCH] NdkMediaFormat_*() parameter checking verify non-null format and name fields in the NdkMediaFormat_set/get functions. Update unit tests to verify fixes are in place. Bug: 243204176 Test: atest libmediandk_test Change-Id: I551b61c96dce71310bcd1d057abad0fde021cc55 --- media/ndk/NdkMediaFormat.cpp | 97 ++++++++++++++++++++++++- media/ndk/tests/NdkMediaFormat_test.cpp | 45 ++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/media/ndk/NdkMediaFormat.cpp b/media/ndk/NdkMediaFormat.cpp index 923453ac1b..a95e87419a 100644 --- a/media/ndk/NdkMediaFormat.cpp +++ b/media/ndk/NdkMediaFormat.cpp @@ -147,38 +147,80 @@ const char* AMediaFormat_toString(AMediaFormat *mData) { EXPORT bool AMediaFormat_getInt32(AMediaFormat* format, const char *name, int32_t *out) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findInt32(name, out); } EXPORT bool AMediaFormat_getInt64(AMediaFormat* format, const char *name, int64_t *out) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findInt64(name, out); } EXPORT bool AMediaFormat_getFloat(AMediaFormat* format, const char *name, float *out) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findFloat(name, out); } EXPORT bool AMediaFormat_getDouble(AMediaFormat* format, const char *name, double *out) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findDouble(name, out); } EXPORT bool AMediaFormat_getSize(AMediaFormat* format, const char *name, size_t *out) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findSize(name, out); } EXPORT bool AMediaFormat_getRect(AMediaFormat* format, const char *name, int32_t *left, int32_t *top, int32_t *right, int32_t *bottom) { + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } return format->mFormat->findRect(name, left, top, right, bottom); } EXPORT bool AMediaFormat_getBuffer(AMediaFormat* format, const char *name, void** data, size_t *outsize) { sp buf; + if (format == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } if (format->mFormat->findBuffer(name, &buf)) { *data = buf->data() + buf->offset(); *outsize = buf->size(); @@ -189,7 +231,12 @@ bool AMediaFormat_getBuffer(AMediaFormat* format, const char *name, void** data, EXPORT bool AMediaFormat_getString(AMediaFormat* mData, const char *name, const char **out) { - + if (mData == nullptr) { + return false; + } + if (name == nullptr) { + return false; + } for (size_t i = 0; i < mData->mStringCache.size(); i++) { if (strcmp(mData->mStringCache.keyAt(i).string(), name) == 0) { mData->mStringCache.removeItemsAt(i, 1); @@ -212,43 +259,91 @@ bool AMediaFormat_getString(AMediaFormat* mData, const char *name, const char ** EXPORT void AMediaFormat_setInt32(AMediaFormat* format, const char *name, int32_t value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setInt32(name, value); } EXPORT void AMediaFormat_setInt64(AMediaFormat* format, const char *name, int64_t value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setInt64(name, value); } EXPORT void AMediaFormat_setFloat(AMediaFormat* format, const char* name, float value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setFloat(name, value); } EXPORT void AMediaFormat_setDouble(AMediaFormat* format, const char* name, double value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setDouble(name, value); } EXPORT void AMediaFormat_setSize(AMediaFormat* format, const char* name, size_t value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setSize(name, value); } EXPORT void AMediaFormat_setRect(AMediaFormat* format, const char *name, int32_t left, int32_t top, int32_t right, int32_t bottom) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } format->mFormat->setRect(name, left, top, right, bottom); } EXPORT void AMediaFormat_setString(AMediaFormat* format, const char* name, const char* value) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } // AMessage::setString() makes a copy of the string format->mFormat->setString(name, value, strlen(value)); } EXPORT void AMediaFormat_setBuffer(AMediaFormat* format, const char* name, const void* data, size_t size) { + if (format == nullptr) { + return; + } + if (name == nullptr) { + return; + } // the ABuffer(void*, size_t) constructor doesn't take ownership of the data, so create // a new buffer and copy the data into it sp buf = new ABuffer(size); diff --git a/media/ndk/tests/NdkMediaFormat_test.cpp b/media/ndk/tests/NdkMediaFormat_test.cpp index 668d0a4463..18690b8782 100644 --- a/media/ndk/tests/NdkMediaFormat_test.cpp +++ b/media/ndk/tests/NdkMediaFormat_test.cpp @@ -51,6 +51,13 @@ TEST(NdkMediaFormat_tests, test_int32) { EXPECT_FALSE(AMediaFormat_getInt64(fmt1, "five", &i64)); EXPECT_EQ(i32, 5); + // verify detecting some bad parameters. + AMediaFormat_setInt32(nullptr, "whatever", 6); + AMediaFormat_setInt32(fmt1, nullptr, 6); + + EXPECT_FALSE(AMediaFormat_getInt32(nullptr, "whatever", &i32)); + EXPECT_FALSE(AMediaFormat_getInt32(fmt1, nullptr, &i32)); + AMediaFormat_delete(fmt1); } @@ -67,6 +74,13 @@ TEST(NdkMediaFormat_tests, test_int64) { EXPECT_FALSE(AMediaFormat_getInt64(fmt1, "five", &i64)); EXPECT_EQ(i64, -1); + // verify detecting some bad parameters. + AMediaFormat_setInt64(nullptr, "whatever", 6); + AMediaFormat_setInt64(fmt1, nullptr, 6); + + EXPECT_FALSE(AMediaFormat_getInt64(nullptr, "whatever", &i64)); + EXPECT_FALSE(AMediaFormat_getInt64(fmt1, nullptr, &i64)); + AMediaFormat_delete(fmt1); } @@ -80,6 +94,13 @@ TEST(NdkMediaFormat_tests, test_size) { EXPECT_TRUE(AMediaFormat_getSize(fmt1, "medium", &size)); EXPECT_EQ(size, 10); + // verify detecting some bad parameters. + AMediaFormat_setSize(nullptr, "whatever", 6); + AMediaFormat_setSize(fmt1, nullptr, 6); + + EXPECT_FALSE(AMediaFormat_getSize(nullptr, "whatever", &size)); + EXPECT_FALSE(AMediaFormat_getSize(fmt1, nullptr, &size)); + AMediaFormat_delete(fmt1); } @@ -90,6 +111,14 @@ TEST(NdkMediaFormat_tests, test_float) { AMediaFormat_setFloat(fmt1, "ship", 0.5); EXPECT_TRUE(AMediaFormat_getFloat(fmt1, "boat", &f)); EXPECT_EQ(f, 1.5); + + // verify detecting some bad parameters. + AMediaFormat_setFloat(nullptr, "whatever", 1.5); + AMediaFormat_setFloat(fmt1, nullptr, 1.5); + + EXPECT_FALSE(AMediaFormat_getFloat(nullptr, "whatever", &f)); + EXPECT_FALSE(AMediaFormat_getFloat(fmt1, nullptr, &f)); + AMediaFormat_delete(fmt1); } @@ -100,6 +129,14 @@ TEST(NdkMediaFormat_tests, test_double) { AMediaFormat_setDouble(fmt1, "dip", 0.5); EXPECT_TRUE(AMediaFormat_getDouble(fmt1, "trouble", &d)); EXPECT_EQ(d, 100.5); + + // verify detecting some bad parameters. + AMediaFormat_setDouble(nullptr, "whatever", 1.5); + AMediaFormat_setDouble(fmt1, nullptr, 1.5); + + EXPECT_FALSE(AMediaFormat_getDouble(nullptr, "whatever", &d)); + EXPECT_FALSE(AMediaFormat_getDouble(fmt1, nullptr, &d)); + AMediaFormat_delete(fmt1); } @@ -111,8 +148,16 @@ TEST(NdkMediaFormat_tests, test_string) { AMediaFormat_setString(fmt1, "stringtheory", content); EXPECT_TRUE(AMediaFormat_getString(fmt1, "stringtheory", &out)); EXPECT_NE(out, nullptr); + EXPECT_NE(out, content); // should not be the original EXPECT_EQ(strcmp(out,content), 0); + // verify detecting some bad parameters. + AMediaFormat_setString(nullptr, "whatever", content); + AMediaFormat_setString(fmt1, nullptr, content); + + EXPECT_FALSE(AMediaFormat_getString(nullptr, "whatever", &out)); + EXPECT_FALSE(AMediaFormat_getString(fmt1, nullptr, &out)); + AMediaFormat_delete(fmt1); }