libbinder: RPC error on unexpected session ID size

The session ID is created by the server, and passed to the client. This
prevents making a OOM-large allocation if a malicious client claims to
pass a large session ID.

Fixes: 201599425
Test: binderRpcTest
Change-Id: I24c66b70fbb5e194c2b603214e2f962b31ff27f9

1 files changed, 17 insertions, 9 deletions

diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index ba2920e3ac..44b588be00 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -16,6 +16,7 @@
 
 #define LOG_TAG "RpcServer"
 
+#include <inttypes.h>
 #include <poll.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -38,6 +39,8 @@
 
 namespace android {
 
+constexpr size_t kSessionIdBytes = 32;
+
 using base::ScopeGuard;
 using base::unique_fd;
 
@@ -289,13 +292,19 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie
     std::vector<uint8_t> sessionId;
     if (status == OK) {
         if (header.sessionIdSize > 0) {
-            sessionId.resize(header.sessionIdSize);
-            status = client->interruptableReadFully(server->mShutdownTrigger.get(),
-                                                    sessionId.data(), sessionId.size(), {});
-            if (status != OK) {
-                ALOGE("Failed to read session ID for client connecting to RPC server: %s",
-                      statusToString(status).c_str());
-                // still need to cleanup before we can return
+            if (header.sessionIdSize == kSessionIdBytes) {
+                sessionId.resize(header.sessionIdSize);
+                status = client->interruptableReadFully(server->mShutdownTrigger.get(),
+                                                        sessionId.data(), sessionId.size(), {});
+                if (status != OK) {
+                    ALOGE("Failed to read session ID for client connecting to RPC server: %s",
+                          statusToString(status).c_str());
+                    // still need to cleanup before we can return
+                }
+            } else {
+                ALOGE("Malformed session ID. Expecting session ID of size %zu but got %" PRIu16,
+                      kSessionIdBytes, header.sessionIdSize);
+                status = BAD_VALUE;
             }
         }
     }
@@ -353,8 +362,7 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie
             // Uniquely identify session at the application layer. Even if a
             // client/server use the same certificates, if they create multiple
             // sessions, we still want to distinguish between them.
-            constexpr size_t kSessionIdSize = 32;
-            sessionId.resize(kSessionIdSize);
+            sessionId.resize(kSessionIdBytes);
             size_t tries = 0;
             do {
                 // don't block if there is some entropy issue