Merge "Increase statsd guardrails for atoms and metrics" into main am: 9b6437d986
Original change: https://android-review.googlesource.com/c/platform/packages/modules/StatsD/+/3023983
Change-Id: Idfddfb50a2b0dee4a5e3d34deb8de4cab6bfbe90
Signed-off-by: Automerger Merge Worker <[email protected]>
diff --git a/lib/libstatssocket/stats_event.c b/lib/libstatssocket/stats_event.c
index ade1b93..1c8aaf2 100644
--- a/lib/libstatssocket/stats_event.c
+++ b/lib/libstatssocket/stats_event.c
@@ -325,6 +325,9 @@
// Side-effect: modifies event->errors if field has too many annotations
static void increment_annotation_count(AStatsEvent* event) {
+ if (event->lastFieldPos >= event->bufSize) {
+ return;
+ }
uint8_t fieldType = event->buf[event->lastFieldPos] & 0x0F;
uint32_t oldAnnotationCount = (event->buf[event->lastFieldPos] & 0xF0) >> 4;
uint32_t newAnnotationCount = oldAnnotationCount + 1;
diff --git a/lib/libstatssocket/tests/stats_event_test.cpp b/lib/libstatssocket/tests/stats_event_test.cpp
index 93a99f1..dea81c2 100644
--- a/lib/libstatssocket/tests/stats_event_test.cpp
+++ b/lib/libstatssocket/tests/stats_event_test.cpp
@@ -536,6 +536,50 @@
AStatsEvent_release(event);
}
+TEST(StatsEventTest, TestHeapBufferOverflowError) {
+ const std::string testString(4039, 'A');
+ const std::string testString2(47135, 'B');
+
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ AStatsEvent_writeString(event, testString.c_str());
+ size_t bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 4060);
+ uint32_t errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors, 0);
+
+ // expand the buffer and fill with data up to the very last byte
+ AStatsEvent_writeString(event, testString2.c_str());
+ bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 50 * 1024);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors, 0);
+
+ // this write is no-op due to buffer reached its max capacity
+ // should set the overflow flag
+ AStatsEvent_writeString(event, testString2.c_str());
+ bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 50 * 1024);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW);
+
+ // here should be crash
+ AStatsEvent_addBoolAnnotation(event, 1, false);
+
+ AStatsEvent_write(event);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW);
+
+ AStatsEvent_release(event);
+}
+
TEST(StatsEventTest, TestPullOverflowError) {
const uint32_t atomId = 10100;
const vector<uint8_t> bytes(430 /* number of elements */, 1 /* value of each element */);
diff --git a/statsd/Android.bp b/statsd/Android.bp
index 2199a5c..4285853 100644
--- a/statsd/Android.bp
+++ b/statsd/Android.bp
@@ -589,7 +589,6 @@
defaults: [
"statsd_defaults",
"service_fuzzer_defaults",
- "fuzzer_disable_leaks",
],
srcs: [
"fuzzers/statsd_service_fuzzer.cpp",
diff --git a/statsd/src/logd/LogEvent.cpp b/statsd/src/logd/LogEvent.cpp
index 94fe5cf..affb080 100644
--- a/statsd/src/logd/LogEvent.cpp
+++ b/statsd/src/logd/LogEvent.cpp
@@ -266,7 +266,7 @@
}
// Assumes that mValues is not empty
-bool LogEvent::checkPreviousValueType(Type expected) {
+bool LogEvent::checkPreviousValueType(Type expected) const {
return mValues[mValues.size() - 1].mValue.getType() == expected;
}
@@ -283,7 +283,7 @@
}
// Allowed types: INT, repeated INT
- if (numElements > mValues.size() || !checkPreviousValueType(INT) ||
+ if (mValues.empty() || numElements > mValues.size() || !checkPreviousValueType(INT) ||
annotationType != BOOL_TYPE) {
VLOG("Atom ID %d error while parseIsUidAnnotation()", mTagId);
mValid = false;
diff --git a/statsd/src/logd/LogEvent.h b/statsd/src/logd/LogEvent.h
index c34d34e..db2bc4e 100644
--- a/statsd/src/logd/LogEvent.h
+++ b/statsd/src/logd/LogEvent.h
@@ -296,7 +296,7 @@
void parseStateNestedAnnotation(uint8_t annotationType, std::optional<uint8_t> numElements);
void parseRestrictionCategoryAnnotation(uint8_t annotationType);
void parseFieldRestrictionAnnotation(uint8_t annotationType);
- bool checkPreviousValueType(Type expected);
+ bool checkPreviousValueType(Type expected) const;
bool getRestrictedMetricsFlag();
/**
diff --git a/statsd/tools/localtools/src/com/android/statsd/shelltools/ExtensionAtomsRegistry.java b/statsd/tools/localtools/src/com/android/statsd/shelltools/ExtensionAtomsRegistry.java
index 3222dbc..0061cd4 100644
--- a/statsd/tools/localtools/src/com/android/statsd/shelltools/ExtensionAtomsRegistry.java
+++ b/statsd/tools/localtools/src/com/android/statsd/shelltools/ExtensionAtomsRegistry.java
@@ -61,7 +61,6 @@
import android.os.statsd.media.MediaCodecExtensionAtoms;
import com.android.os.credentials.CredentialsExtensionAtoms;
import com.android.os.sdksandbox.SdksandboxExtensionAtoms;
-import com.android.os.apex.ApexExtensionAtoms;
import com.google.protobuf.ExtensionRegistry;
@@ -137,6 +136,5 @@
CredentialsExtensionAtoms.registerAllExtensions(extensionRegistry);
SdksandboxExtensionAtoms.registerAllExtensions(extensionRegistry);
ArtExtensionAtoms.registerAllExtensions(extensionRegistry);
- ApexExtensionAtoms.registerAllExtensions(extensionRegistry);
}
}
diff --git a/tests/Android.bp b/tests/Android.bp
index 71784a9..cf12ddf 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -29,6 +29,7 @@
"cts",
"general-tests",
"mts-statsd",
+ "mcts-statsd",
],
libs: [
