diff --git a/security/sandbox/common/test/SandboxTestingChildTests.h b/security/sandbox/common/test/SandboxTestingChildTests.h index 71f642035323d..b676c3eac1459 100644 --- a/security/sandbox/common/test/SandboxTestingChildTests.h +++ b/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 diff --git a/security/sandbox/linux/broker/SandboxBroker.cpp b/security/sandbox/linux/broker/SandboxBroker.cpp index c6f0cdf6ca90c..75ab52979e12a 100644 --- a/security/sandbox/linux/broker/SandboxBroker.cpp +++ b/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(req.mBufSize), ssize_t(0)); + respSize = std::min(respSize, callerSize); + resp.mError = AssertedCast(respSize); ios[1].iov_base = &respBuf; - ios[1].iov_len = respSize; + ios[1].iov_len = ReleaseAssertedCast(respSize); + MOZ_RELEASE_ASSERT(ios[1].iov_len <= sizeof(respBuf)); } else { resp.mError = -errno; } diff --git a/security/sandbox/linux/broker/SandboxBrokerCommon.cpp b/security/sandbox/linux/broker/SandboxBrokerCommon.cpp index 2a9dcfff4004b..baba0b25a5139 100644 --- a/security/sandbox/linux/broker/SandboxBrokerCommon.cpp +++ b/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; }