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
diff --git a/logd/LogListener.cpp b/logd/LogListener.cpp
index d2df68e..fcf2cd8 100644
--- a/logd/LogListener.cpp
+++ b/logd/LogListener.cpp
@@ -43,9 +43,10 @@
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 @@
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);