From c6ed8f39d3e4c45e83923dac93c1388526c99a3d Mon Sep 17 00:00:00 2001
From: Paul Elliott <paul.elliott@arm.com>
Date: Tue, 7 Nov 2017 11:11:41 +0000
Subject: [PATCH] Buffer overrun in __android_log_is_loggable() fix

Fix for buffer overrun when a tag that is too big is sent to logd.
Buffer supplied is precisely the right size for max message length
however strlen will be run on the buffer, so need to ensure null
terminator, otherwise any strlen will go off the end of the buffer.
Also converted LogBuffer::Log() over to use the safer strnlen in the
case where it is measuring the buffer (and converted over to using
__android_log_is_loggable_len())

Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Test: liblog.android_log_buf_print__maxtag
Change-Id: I3cb8b25af55943fb0f4658657560eb2300f52961
---
 logd/LogBuffer.cpp   |  8 +++++++-
 logd/LogListener.cpp | 10 +++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp
index 749832530..43362fb5a 100644
--- a/logd/LogBuffer.cpp
+++ b/logd/LogBuffer.cpp
@@ -212,13 +212,19 @@ int LogBuffer::log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,
     if (log_id != LOG_ID_SECURITY) {
         int prio = ANDROID_LOG_INFO;
         const char* tag = nullptr;
+        size_t tag_len = 0;
         if (log_id == LOG_ID_EVENTS) {
             tag = tagToName(elem->getTag());
+            if (tag) {
+                tag_len = strlen(tag);
+            }
         } else {
             prio = *msg;
             tag = msg + 1;
+            tag_len = strnlen(tag, len - 1);
         }
-        if (!__android_log_is_loggable(prio, tag, ANDROID_LOG_VERBOSE)) {
+        if (!__android_log_is_loggable_len(prio, tag, tag_len,
+                                           ANDROID_LOG_VERBOSE)) {
             // Log traffic received to total
             wrlock();
             stats.addTotal(elem);
diff --git a/logd/LogListener.cpp b/logd/LogListener.cpp
index d2df68eef..fcf2cd8a1 100644
--- a/logd/LogListener.cpp
+++ b/logd/LogListener.cpp
@@ -43,9 +43,10 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
         name_set = true;
     }
 
+    // + 1 to ensure null terminator if MAX_PAYLOAD buffer is received
     char buffer[sizeof_log_id_t + sizeof(uint16_t) + sizeof(log_time) +
-                LOGGER_ENTRY_MAX_PAYLOAD];
-    struct iovec iov = { buffer, sizeof(buffer) };
+                LOGGER_ENTRY_MAX_PAYLOAD + 1];
+    struct iovec iov = { buffer, sizeof(buffer) - 1 };
 
     alignas(4) char control[CMSG_SPACE(sizeof(struct ucred))];
     struct msghdr hdr = {
@@ -55,13 +56,16 @@ bool LogListener::onDataAvailable(SocketClient* cli) {
     int socket = cli->getSocket();
 
     // To clear the entire buffer is secure/safe, but this contributes to 1.68%
-    // overhead under logging load. We are safe because we check counts.
+    // overhead under logging load. We are safe because we check counts, but
+    // still need to clear null terminator
     // memset(buffer, 0, sizeof(buffer));
     ssize_t n = recvmsg(socket, &hdr, 0);
     if (n <= (ssize_t)(sizeof(android_log_header_t))) {
         return false;
     }
 
+    buffer[n] = 0;
+
     struct ucred* cred = NULL;
 
     struct cmsghdr* cmsg = CMSG_FIRSTHDR(&hdr);