From 470484d2a25ad432190a01d1c763b4b36db33c7e Mon Sep 17 00:00:00 2001 From: Connor O'Brien Date: Fri, 12 Aug 2016 11:52:46 -0700 Subject: [PATCH] Fix vold vulnerability in FrameworkListener Modify FrameworkListener to ignore commands that exceed the maximum buffer length and send an error message. Bug: 29831647 Change-Id: I9e57d1648d55af2ca0191bb47868e375ecc26950 Signed-off-by: Connor O'Brien (cherry picked from commit baa126dc158a40bc83c17c6d428c760e5b93fb1a) --- include/sysutils/FrameworkListener.h | 1 + libsysutils/src/FrameworkListener.cpp | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/sysutils/FrameworkListener.h b/include/sysutils/FrameworkListener.h index f1a4b43f8..6cbe831f9 100644 --- a/include/sysutils/FrameworkListener.h +++ b/include/sysutils/FrameworkListener.h @@ -32,6 +32,7 @@ private: int mCommandCount; bool mWithSeq; FrameworkCommandCollection *mCommands; + bool mSkipToNextNullByte; public: FrameworkListener(const char *socketName); diff --git a/libsysutils/src/FrameworkListener.cpp b/libsysutils/src/FrameworkListener.cpp index 02a401d41..e3d67b331 100644 --- a/libsysutils/src/FrameworkListener.cpp +++ b/libsysutils/src/FrameworkListener.cpp @@ -42,6 +42,7 @@ void FrameworkListener::init(const char *socketName, bool withSeq) { errorRate = 0; mCommandCount = 0; mWithSeq = withSeq; + mSkipToNextNullByte = false; } bool FrameworkListener::onDataAvailable(SocketClient *c) { @@ -52,10 +53,15 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) { if (len < 0) { SLOGE("read() failed (%s)", strerror(errno)); return false; - } else if (!len) + } else if (!len) { return false; - if(buffer[len-1] != '\0') + } else if (buffer[len-1] != '\0') { SLOGW("String is not zero-terminated"); + android_errorWriteLog(0x534e4554, "29831647"); + c->sendMsg(500, "Command too large for buffer", false); + mSkipToNextNullByte = true; + return false; + } int offset = 0; int i; @@ -63,11 +69,16 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) { for (i = 0; i < len; i++) { if (buffer[i] == '\0') { /* IMPORTANT: dispatchCommand() expects a zero-terminated string */ - dispatchCommand(c, buffer + offset); + if (mSkipToNextNullByte) { + mSkipToNextNullByte = false; + } else { + dispatchCommand(c, buffer + offset); + } offset = i + 1; } } + mSkipToNextNullByte = false; return true; }