Bug 1855341 - Fix Linux sandbox broker readlink emulation when the re…

…sult is truncated. r=gcp

Normally, when `readlink` is called with a buffer that's smaller than
the symlink target, the string will be silently truncated and `readlink`
returns the number of bytes actually written (== the buffer size); the
caller is expected to detect this and enlarge the buffer as needed.

In the broker protocol, the client sends the buffer size in the request,
and then reads the response directly into the buffer.  The bug is that
the broker ignores the requested size and sends the entire response; as
a result, the response is truncated by `recvmsg` and the `MSG_TRUNC`
flag is set.  We don't expect `MSG_TRUNC`, so this either sets off a
diagnostic assertion or else fails with the implausible error `EMFILE`
("Too many open files", because it's treated the same as `MSG_CTRUNC`).

The fix simply is for the broker to respect the client's advertised
buffer size as intended; the error handling is also improved.

Note that `EMSGSIZE` is not used as a pseudo-error here, even though
"Message too long" might make sense as an error message, to avoid
confusion with the real `EMSGSIZE` that can be returned by `sendmsg`.

Differential Revision: https://phabricator.services.mozilla.com/D192530

security/sandbox/common/test/SandboxTestingChildTests.h

@@ -507,6 +507,25 @@ void RunTestsContent(SandboxTestingChild* child) {
     return realpath("/etc/localtime", buf) ? 0 : -1;
   });
 
+  // Check that readlink truncates results longer than the buffer
+  // (rather than failing) and returns the total number of bytes
+  // actually written (not the size of the link or anything else).
+  {
+    char buf;
+    ssize_t rv = readlink("/etc/localtime", &buf, 1);
+    int err = errno;
+    if (rv == 1) {
+      child->SendReportTestResults("readlink truncate"_ns, true,
+                                   "expected 1, got 1"_ns);
+    } else if (rv < 0) {
+      nsPrintfCString msg("expected 1, got error: %s", strerror(err));
+      child->SendReportTestResults("readlink truncate"_ns, false, msg);
+    } else {
+      nsPrintfCString msg("expected 1, got %zd", rv);
+      child->SendReportTestResults("readlink truncate"_ns, false, msg);
+    }
+  }
+
 #  endif  // XP_LINUX
 
 #  ifdef XP_MACOSX

security/sandbox/linux/broker/SandboxBroker.cpp

@@ -1002,9 +1002,16 @@ void SandboxBroker::ThreadMain(void) {
                   free(resolvedBuf);
                 }
               }
-              resp.mError = respSize;
+              // Truncate the reply to the size of the client's
+              // buffer, matching the real readlink()'s behavior in
+              // that case, and being careful with the input data.
+              ssize_t callerSize =
+                  std::max(AssertedCast<ssize_t>(req.mBufSize), ssize_t(0));
+              respSize = std::min(respSize, callerSize);
+              resp.mError = AssertedCast<int>(respSize);
               ios[1].iov_base = &respBuf;
-              ios[1].iov_len = respSize;
+              ios[1].iov_len = ReleaseAssertedCast<size_t>(respSize);
+              MOZ_RELEASE_ASSERT(ios[1].iov_len <= sizeof(respBuf));
             } else {
               resp.mError = -errno;
             }

security/sandbox/linux/broker/SandboxBrokerCommon.cpp

@@ -97,7 +97,7 @@ ssize_t SandboxBrokerCommon::RecvWithFd(int aFd, const iovec* aIO,
         // giving us an empty one, if errors prevent transferring the
         // fd.
         MOZ_DIAGNOSTIC_ASSERT(cmsg->cmsg_len != 0);
-        errno = EMSGSIZE;
+        errno = EPROTO;
         return -1;
       }
       *aPassedFdPtr = fds[0];
@@ -111,14 +111,18 @@ ssize_t SandboxBrokerCommon::RecvWithFd(int aFd, const iovec* aIO,
       close(*aPassedFdPtr);
       *aPassedFdPtr = -1;
     }
-    // MSG_CTRUNC usually means the fd was dropped due to fd
+    // MSG_CTRUNC usually means the attached fd was dropped due to fd
     // exhaustion in the receiving process, so map that to `EMFILE`.
+    // (It could also happen if the other process maliciously sends
+    // too many fds.)
+    //
     // MSG_TRUNC (truncation of the data part) shouldn't ever happen.
-    // (If the sender is malicious it can send too many bytes or fds,
-    // but this is about getting an accurate error message in genuine
-    // error cases.)
+    // However, it has happened in the past, due to accidentally
+    // sending more data than the receiver was expecting.  We assert
+    // that that doesn't happen (and, if it does, try to map it to a
+    // vaguely sensible error code).
     MOZ_DIAGNOSTIC_ASSERT((msg.msg_flags & MSG_TRUNC) == 0);
-    errno = EMFILE;
+    errno = (msg.msg_flags & MSG_CTRUNC) ? EMFILE : EPROTO;
     return -1;
   }