Skip to content
Snippets Groups Projects
Commit 771ab014 authored by Connor O'Brien's avatar Connor O'Brien Committed by gitbuildkicker
Browse files

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: default avatarConnor O'Brien <connoro@google.com>
(cherry picked from commit baa126dc)
(cherry picked from commit 470484d2)
parent ecf5fd58
Branches
Tags
No related merge requests found
...@@ -32,6 +32,7 @@ private: ...@@ -32,6 +32,7 @@ private:
int mCommandCount; int mCommandCount;
bool mWithSeq; bool mWithSeq;
FrameworkCommandCollection *mCommands; FrameworkCommandCollection *mCommands;
bool mSkipToNextNullByte;
public: public:
FrameworkListener(const char *socketName); FrameworkListener(const char *socketName);
......
...@@ -49,6 +49,7 @@ void FrameworkListener::init(const char *socketName UNUSED, bool withSeq) { ...@@ -49,6 +49,7 @@ void FrameworkListener::init(const char *socketName UNUSED, bool withSeq) {
errorRate = 0; errorRate = 0;
mCommandCount = 0; mCommandCount = 0;
mWithSeq = withSeq; mWithSeq = withSeq;
mSkipToNextNullByte = false;
} }
bool FrameworkListener::onDataAvailable(SocketClient *c) { bool FrameworkListener::onDataAvailable(SocketClient *c) {
...@@ -59,10 +60,15 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) { ...@@ -59,10 +60,15 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) {
if (len < 0) { if (len < 0) {
SLOGE("read() failed (%s)", strerror(errno)); SLOGE("read() failed (%s)", strerror(errno));
return false; return false;
} else if (!len) } else if (!len) {
return false; return false;
if(buffer[len-1] != '\0') } else if (buffer[len-1] != '\0') {
SLOGW("String is not zero-terminated"); 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 offset = 0;
int i; int i;
...@@ -70,11 +76,16 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) { ...@@ -70,11 +76,16 @@ bool FrameworkListener::onDataAvailable(SocketClient *c) {
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
if (buffer[i] == '\0') { if (buffer[i] == '\0') {
/* IMPORTANT: dispatchCommand() expects a zero-terminated string */ /* IMPORTANT: dispatchCommand() expects a zero-terminated string */
if (mSkipToNextNullByte) {
mSkipToNextNullByte = false;
} else {
dispatchCommand(c, buffer + offset); dispatchCommand(c, buffer + offset);
}
offset = i + 1; offset = i + 1;
} }
} }
mSkipToNextNullByte = false;
return true; return true;
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment