summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Muhammad Qureshi <muhammadq@google.com> 2021-01-30 02:30:06 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> 2021-01-30 02:30:06 +0000
commit60b00ec30f4f4145027966aead07a98bf421b23b (patch)
tree601c7e1ee5bbce8948ab3fe933ec70f73fbff114
parent536ca6cad71c3305a0cc5d792029ab69fa4c3168 (diff)
parent53251a491faab1fb88e28b7cafe5913842b8d71d (diff)
[RESTRICT AUTOMERGE] Fix potential out of bounds writes in LogEvent. am: 53251a491f
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/13321771 MUST ONLY BE SUBMITTED BY AUTOMERGER Change-Id: I1da4a9159e5611d80c618913f728defb5399d927
-rw-r--r--cmds/statsd/src/logd/LogEvent.cpp27
-rw-r--r--cmds/statsd/tests/LogEvent_test.cpp110
2 files changed, 133 insertions, 4 deletions
diff --git a/cmds/statsd/src/logd/LogEvent.cpp b/cmds/statsd/src/logd/LogEvent.cpp
index f56fa6221bc9..4f031724763f 100644
--- a/cmds/statsd/src/logd/LogEvent.cpp
+++ b/cmds/statsd/src/logd/LogEvent.cpp
@@ -19,6 +19,7 @@
#include <android-base/stringprintf.h>
#include <android/binder_ibinder.h>
+#include <log/log.h>
#include <private/android_filesystem_config.h>
#include "annotations.h"
@@ -216,13 +217,18 @@ void LogEvent::parseAttributionChain(int32_t* pos, int32_t depth, bool* last,
last[2] = true;
parseString(pos, /*depth=*/2, last, /*numAnnotations=*/0);
}
- // Check if at least one node was successfully parsed.
- if (mValues.size() - 1 > firstUidInChainIndex) {
+
+ if (mValues.size() - 1 > INT8_MAX) {
+ mValid = false;
+ } else if (mValues.size() - 1 > firstUidInChainIndex) {
+ // At least one node was successfully parsed.
mAttributionChainStartIndex = static_cast<int8_t>(firstUidInChainIndex);
mAttributionChainEndIndex = static_cast<int8_t>(mValues.size() - 1);
}
- parseAnnotations(numAnnotations, firstUidInChainIndex);
+ if (mValid) {
+ parseAnnotations(numAnnotations, firstUidInChainIndex);
+ }
pos[1] = pos[2] = 1;
last[1] = last[2] = false;
@@ -234,7 +240,8 @@ bool LogEvent::checkPreviousValueType(Type expected) {
}
void LogEvent::parseIsUidAnnotation(uint8_t annotationType) {
- if (mValues.empty() || !checkPreviousValueType(INT) || annotationType != BOOL_TYPE) {
+ if (mValues.empty() || mValues.size() - 1 > INT8_MAX || !checkPreviousValueType(INT)
+ || annotationType != BOOL_TYPE) {
mValid = false;
return;
}
@@ -270,6 +277,12 @@ void LogEvent::parsePrimaryFieldFirstUidAnnotation(uint8_t annotationType,
return;
}
+ if (static_cast<int>(mValues.size() - 1) < firstUidInChainIndex) { // AttributionChain is empty.
+ mValid = false;
+ android_errorWriteLog(0x534e4554, "174485572");
+ return;
+ }
+
const bool primaryField = readNextValue<uint8_t>();
mValues[firstUidInChainIndex].mAnnotations.setPrimaryField(primaryField);
}
@@ -280,6 +293,12 @@ void LogEvent::parseExclusiveStateAnnotation(uint8_t annotationType) {
return;
}
+ if (mValues.size() - 1 > INT8_MAX) {
+ android_errorWriteLog(0x534e4554, "174488848");
+ mValid = false;
+ return;
+ }
+
const bool exclusiveState = readNextValue<uint8_t>();
mExclusiveStateFieldIndex = static_cast<int8_t>(mValues.size() - 1);
mValues[getExclusiveStateFieldIndex()].mAnnotations.setExclusiveState(exclusiveState);
diff --git a/cmds/statsd/tests/LogEvent_test.cpp b/cmds/statsd/tests/LogEvent_test.cpp
index 5c170c07eb7d..aed25475da11 100644
--- a/cmds/statsd/tests/LogEvent_test.cpp
+++ b/cmds/statsd/tests/LogEvent_test.cpp
@@ -363,6 +363,116 @@ TEST(LogEventTest, TestResetStateAnnotation) {
EXPECT_EQ(event.getResetState(), resetState);
}
+TEST(LogEventTest, TestExclusiveStateAnnotationAfterTooManyFields) {
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ const unsigned int numAttributionNodes = 64;
+
+ uint32_t uids[numAttributionNodes];
+ const char* tags[numAttributionNodes];
+
+ for (unsigned int i = 1; i <= numAttributionNodes; i++) {
+ uids[i-1] = i;
+ tags[i-1] = std::to_string(i).c_str();
+ }
+
+ AStatsEvent_writeAttributionChain(event, uids, tags, numAttributionNodes);
+ AStatsEvent_writeInt32(event, 1);
+ AStatsEvent_addBoolAnnotation(event, ANNOTATION_ID_EXCLUSIVE_STATE, true);
+
+ AStatsEvent_build(event);
+
+ size_t size;
+ uint8_t* buf = AStatsEvent_getBuffer(event, &size);
+
+ LogEvent logEvent(/*uid=*/1000, /*pid=*/1001);
+ EXPECT_FALSE(logEvent.parseBuffer(buf, size));
+ EXPECT_EQ(-1, logEvent.getExclusiveStateFieldIndex());
+
+ AStatsEvent_release(event);
+}
+
+TEST(LogEventTest, TestUidAnnotationAfterTooManyFields) {
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ const unsigned int numAttributionNodes = 64;
+
+ uint32_t uids[numAttributionNodes];
+ const char* tags[numAttributionNodes];
+
+ for (unsigned int i = 1; i <= numAttributionNodes; i++) {
+ uids[i-1] = i;
+ tags[i-1] = std::to_string(i).c_str();
+ }
+
+ AStatsEvent_writeAttributionChain(event, uids, tags, numAttributionNodes);
+ AStatsEvent_writeInt32(event, 1);
+ AStatsEvent_addBoolAnnotation(event, ANNOTATION_ID_IS_UID, true);
+
+ AStatsEvent_build(event);
+
+ size_t size;
+ uint8_t* buf = AStatsEvent_getBuffer(event, &size);
+
+ LogEvent logEvent(/*uid=*/1000, /*pid=*/1001);
+ EXPECT_FALSE(logEvent.parseBuffer(buf, size));
+ EXPECT_EQ(-1, logEvent.getUidFieldIndex());
+
+ AStatsEvent_release(event);
+}
+
+TEST(LogEventTest, TestAttributionChainEndIndexAfterTooManyFields) {
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ const unsigned int numAttributionNodes = 65;
+
+ uint32_t uids[numAttributionNodes];
+ const char* tags[numAttributionNodes];
+
+ for (unsigned int i = 1; i <= numAttributionNodes; i++) {
+ uids[i-1] = i;
+ tags[i-1] = std::to_string(i).c_str();
+ }
+
+ AStatsEvent_writeAttributionChain(event, uids, tags, numAttributionNodes);
+
+ AStatsEvent_build(event);
+
+ size_t size;
+ uint8_t* buf = AStatsEvent_getBuffer(event, &size);
+
+ LogEvent logEvent(/*uid=*/1000, /*pid=*/1001);
+ EXPECT_FALSE(logEvent.parseBuffer(buf, size));
+ EXPECT_FALSE(logEvent.hasAttributionChain());
+
+ AStatsEvent_release(event);
+}
+
+TEST(LogEventTest, TestEmptyAttributionChainWithPrimaryFieldFirstUidAnnotation) {
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ uint32_t uids[] = {};
+ const char* tags[] = {};
+
+ AStatsEvent_writeInt32(event, 10);
+ AStatsEvent_writeAttributionChain(event, uids, tags, 0);
+ AStatsEvent_addBoolAnnotation(event, ANNOTATION_ID_PRIMARY_FIELD_FIRST_UID, true);
+
+ AStatsEvent_build(event);
+
+ size_t size;
+ uint8_t* buf = AStatsEvent_getBuffer(event, &size);
+
+ LogEvent logEvent(/*uid=*/1000, /*pid=*/1001);
+ EXPECT_FALSE(logEvent.parseBuffer(buf, size));
+
+ AStatsEvent_release(event);
+}
+
} // namespace statsd
} // namespace os
} // namespace android