Merge "logger: validate hdr_size field in logger entry"
am: fcf7ab8b1b
Change-Id: Ic88fe616256ab4949040771d3bed1dfb823ab981
diff --git a/debuggerd/tombstone.cpp b/debuggerd/tombstone.cpp
index a56f34d..3ddd2b0 100644
--- a/debuggerd/tombstone.cpp
+++ b/debuggerd/tombstone.cpp
@@ -544,6 +544,10 @@
if (!hdr_size) {
hdr_size = sizeof(log_entry.entry_v1);
}
+ if ((hdr_size < sizeof(log_entry.entry_v1)) ||
+ (hdr_size > sizeof(log_entry.entry))) {
+ continue;
+ }
char* msg = reinterpret_cast<char*>(log_entry.buf) + hdr_size;
char timeBuf[32];
diff --git a/include/log/logger.h b/include/log/logger.h
index 60d47a2..256fdd1 100644
--- a/include/log/logger.h
+++ b/include/log/logger.h
@@ -143,7 +143,14 @@
}
char *msg()
{
- return entry.hdr_size ? (char *) buf + entry.hdr_size : entry_v1.msg;
+ unsigned short hdr_size = entry.hdr_size;
+ if (!hdr_size) {
+ hdr_size = sizeof(entry_v1);
+ }
+ if ((hdr_size < sizeof(entry_v1)) || (hdr_size > sizeof(entry))) {
+ return NULL;
+ }
+ return (char *) buf + hdr_size;
}
unsigned int len()
{
diff --git a/liblog/logger_read.c b/liblog/logger_read.c
index 00157b7..0d55453 100644
--- a/liblog/logger_read.c
+++ b/liblog/logger_read.c
@@ -367,6 +367,10 @@
if (log_msg->entry_v2.hdr_size == 0) {
log_msg->entry_v2.hdr_size = sizeof(struct logger_entry);
}
+ if ((log_msg->entry_v2.hdr_size < sizeof(log_msg->entry_v1)) ||
+ (log_msg->entry_v2.hdr_size > sizeof(log_msg->entry))) {
+ return -EINVAL;
+ }
/* len validation */
if (ret <= log_msg->entry_v2.hdr_size) {
diff --git a/liblog/logprint.c b/liblog/logprint.c
index 59bd479..e21a9c4 100644
--- a/liblog/logprint.c
+++ b/liblog/logprint.c
@@ -496,6 +496,11 @@
char *msg = buf->msg;
struct logger_entry_v2 *buf2 = (struct logger_entry_v2 *)buf;
if (buf2->hdr_size) {
+ if ((buf2->hdr_size < sizeof(((struct log_msg *)NULL)->entry_v1)) ||
+ (buf2->hdr_size > sizeof(((struct log_msg *)NULL)->entry))) {
+ fprintf(stderr, "+++ LOG: entry illegal hdr_size\n");
+ return -1;
+ }
msg = ((char *)buf2) + buf2->hdr_size;
if (buf2->hdr_size >= sizeof(struct logger_entry_v4)) {
entry->uid = ((struct logger_entry_v4 *)buf)->uid;
@@ -775,6 +780,11 @@
eventData = (const unsigned char*) buf->msg;
struct logger_entry_v2 *buf2 = (struct logger_entry_v2 *)buf;
if (buf2->hdr_size) {
+ if ((buf2->hdr_size < sizeof(((struct log_msg *)NULL)->entry_v1)) ||
+ (buf2->hdr_size > sizeof(((struct log_msg *)NULL)->entry))) {
+ fprintf(stderr, "+++ LOG: entry illegal hdr_size\n");
+ return -1;
+ }
eventData = ((unsigned char *)buf2) + buf2->hdr_size;
if ((buf2->hdr_size >= sizeof(struct logger_entry_v3)) &&
(((struct logger_entry_v3 *)buf)->lid == LOG_ID_SECURITY)) {
diff --git a/liblog/pmsg_reader.c b/liblog/pmsg_reader.c
index a4eec65..679c159 100644
--- a/liblog/pmsg_reader.c
+++ b/liblog/pmsg_reader.c
@@ -343,6 +343,10 @@
char *msg = (char *)&transp.logMsg + hdr_size;
char *split = NULL;
+ if ((hdr_size < sizeof(transp.logMsg.entry_v1)) ||
+ (hdr_size > sizeof(transp.logMsg.entry))) {
+ continue;
+ }
/* Check for invalid sequence number */
if ((transp.logMsg.entry.nsec % ANDROID_LOG_PMSG_FILE_SEQUENCE) ||
((transp.logMsg.entry.nsec / ANDROID_LOG_PMSG_FILE_SEQUENCE) >=
diff --git a/liblog/tests/liblog_benchmark.cpp b/liblog/tests/liblog_benchmark.cpp
index 1b66a56..f4e3089 100644
--- a/liblog/tests/liblog_benchmark.cpp
+++ b/liblog/tests/liblog_benchmark.cpp
@@ -542,7 +542,7 @@
char* eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
log_time tx(eventData + 4 + 1);
@@ -622,7 +622,7 @@
char* eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
log_time tx(eventData + 4 + 1);
diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp
index df2c766..8f6f07a 100644
--- a/liblog/tests/liblog_test.cpp
+++ b/liblog/tests/liblog_test.cpp
@@ -154,7 +154,7 @@
char *eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
@@ -227,7 +227,7 @@
char *eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_STRING) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_STRING)) {
continue;
}
@@ -506,7 +506,7 @@
char *eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
@@ -637,7 +637,7 @@
char *eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
@@ -788,7 +788,7 @@
char *eventData = log_msg.msg();
- if (eventData[4] != EVENT_TYPE_LONG) {
+ if (!eventData || (eventData[4] != EVENT_TYPE_LONG)) {
continue;
}
@@ -990,9 +990,9 @@
continue;
}
- char *data = log_msg.msg() + 1;
+ char *data = log_msg.msg();
- if (strcmp(data, tag)) {
+ if (!data || strcmp(++data, tag)) {
continue;
}
@@ -1107,9 +1107,9 @@
continue;
}
- char *data = log_msg.msg() + 1;
+ char *data = log_msg.msg();
- if (strcmp(data, tag)) {
+ if (!data || strcmp(++data, tag)) {
continue;
}
@@ -1606,6 +1606,9 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
// Tag
int tag = get4LE(eventData);
@@ -1687,6 +1690,10 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
+
char *original = eventData;
// Tag
@@ -1774,6 +1781,9 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
// Tag
int tag = get4LE(eventData);
@@ -1817,6 +1827,9 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
// Tag
int tag = get4LE(eventData);
@@ -1904,6 +1917,9 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
// Tag
int tag = get4LE(eventData);
@@ -1961,6 +1977,9 @@
}
char *eventData = log_msg.msg();
+ if (!eventData) {
+ continue;
+ }
// Tag
int tag = get4LE(eventData);
@@ -2449,16 +2468,19 @@
android_log_format_free(logformat);
// test buffer reading API
- snprintf(msgBuf, sizeof(msgBuf), "I/[%d]", get4LE(eventData));
- print_barrier();
- fprintf(stderr, "%-10s(%5u): ", msgBuf, pid);
- memset(msgBuf, 0, sizeof(msgBuf));
- int buffer_to_string = android_log_buffer_to_string(
- eventData + sizeof(uint32_t),
- log_msg.entry.len - sizeof(uint32_t),
- msgBuf, sizeof(msgBuf));
- fprintf(stderr, "%s\n", msgBuf);
- print_barrier();
+ int buffer_to_string = -1;
+ if (eventData) {
+ snprintf(msgBuf, sizeof(msgBuf), "I/[%d]", get4LE(eventData));
+ print_barrier();
+ fprintf(stderr, "%-10s(%5u): ", msgBuf, pid);
+ memset(msgBuf, 0, sizeof(msgBuf));
+ buffer_to_string = android_log_buffer_to_string(
+ eventData + sizeof(uint32_t),
+ log_msg.entry.len - sizeof(uint32_t),
+ msgBuf, sizeof(msgBuf));
+ fprintf(stderr, "%s\n", msgBuf);
+ print_barrier();
+ }
EXPECT_EQ(0, buffer_to_string);
EXPECT_EQ(strlen(expected_string), strlen(msgBuf));
EXPECT_EQ(0, strcmp(expected_string, msgBuf));
diff --git a/logd/tests/logd_test.cpp b/logd/tests/logd_test.cpp
index 2014374..301ede9 100644
--- a/logd/tests/logd_test.cpp
+++ b/logd/tests/logd_test.cpp
@@ -213,9 +213,15 @@
version = 1;
break;
- case sizeof(msg->entry_v2):
+ case sizeof(msg->entry_v2): /* PLUS case sizeof(msg->entry_v3): */
if (version == 0) {
- version = 2;
+ version = (msg->entry_v3.lid < LOG_ID_MAX) ? 3 : 2;
+ }
+ break;
+
+ case sizeof(msg->entry_v4):
+ if (version == 0) {
+ version = 4;
}
break;
}
@@ -269,6 +275,11 @@
unsigned int len = msg->entry.len;
fprintf(stderr, "msg[%u]={", len);
unsigned char *cp = reinterpret_cast<unsigned char *>(msg->msg());
+ if (!cp) {
+ static const unsigned char garbage[] = "<INVALID>";
+ cp = const_cast<unsigned char *>(garbage);
+ len = strlen(reinterpret_cast<const char *>(garbage));
+ }
while(len) {
unsigned char *p = cp;
while (*p && (((' ' <= *p) && (*p < 0x7F)) || (*p == '\n'))) {