Firefox · Security/NSS
CVE-2026-117893
Logic Error in Security/NSS
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifsecurity/manager/ssl/NSSSocketControl.h |
modified | |
ifsecurity/manager/ssl/nsNSSIOLayer.cpp |
modified |
Files Changed
security/manager/ssl/NSSSocketControl.cppsecurity/manager/ssl/NSSSocketControl.hsecurity/manager/ssl/nsNSSIOLayer.cpp
Patch
diff --git a/security/manager/ssl/NSSSocketControl.cpp b/security/manager/ssl/NSSSocketControl.cpp
index 47aa645e9b4..8f06cab1676 100644
--- a/security/manager/ssl/NSSSocketControl.cpp
+++ b/security/manager/ssl/NSSSocketControl.cpp
@@ -43,9 +43,7 @@ NSSSocketControl::NSSSocketControl(
mDrewResumptionToken(false),
mNotedTimeUntilReady(false),
mEchExtensionStatus(EchExtensionStatus::kNotPresent),
- mIsShortWritePending(false),
- mShortWritePendingByte(0),
- mShortWriteOriginalAmount(-1),
+ mMaybeShortWrite(Nothing()),
mKEAUsed(nsITLSSocketControl::KEY_EXCHANGE_UNKNOWN),
mKEAKeyBits(0),
mMACAlgorithmUsed(nsITLSSocketControl::SSL_MAC_UNKNOWN),
diff --git a/security/manager/ssl/NSSSocketControl.h b/security/manager/ssl/NSSSocketControl.h
index f8a554034c1..7b66553ab14 100644
--- a/security/manager/ssl/NSSSocketControl.h
+++ b/security/manager/ssl/NSSSocketControl.h
@@ -10,6 +10,7 @@
#include "TLSClientAuthCertSelection.h"
#include "mozilla/Casting.h"
#include "mozilla/Maybe.h"
+#include "mozilla/Span.h"
#include "nsNSSIOLayer.h"
#include "nsThreadUtils.h"
@@ -215,50 +216,51 @@ class NSSSocketControl final : public CommonSocketControl {
mMACAlgorithmUsed = mac;
}
- void SetShortWritePending(int32_t amount, unsigned char data) {
+ void SetShortWritePending(const void* data, size_t amount) {
COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- mIsShortWritePending = true;
- mShortWriteOriginalAmount = amount;
- mShortWritePendingByte = data;
+ MOZ_ASSERT(mMaybeShortWrite.isNothing());
+ nsTArray<uint8_t> shortWrite(amount);
+ shortWrite.AppendElements(
+ reinterpret_cast<uint8_t*>(const_cast<void*>(data)), amount);
+ mMaybeShortWrite.emplace(std::move(shortWrite));
}
bool IsShortWritePending() {
COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- return mIsShortWritePending;
+ return mMaybeShortWrite.isSome();
}
- unsigned char const* GetShortWritePendingByteRef() {
+ size_t ResetShortWritePending() {
COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- return &mShortWritePendingByte;
+ MOZ_ASSERT(mMaybeShortWrite.isSome());
+ size_t shortWriteLength = mMaybeShortWrite->Length();
+ mMaybeShortWrite.reset();
+ return shortWriteLength;
}
- int32_t ResetShortWritePending() {
+ // Ensure that the caller is trying to send the same data as it was
+ // previously when we hit the short-write. To do otherwise would be an error.
+ const void* CheckShortWriteBuffer(const void* data, int32_t amount) {
COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- mIsShortWritePending = false;
- return mShortWriteOriginalAmount;
- }
-
-#ifdef DEBUG
- // These helpers assert that the caller does try to send the same data
- // as it was previously when we hit the short-write. This is a measure
- // to make sure we communicate correctly to the consumer.
- void RememberShortWrittenBuffer(const unsigned char* data) {
- COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- mShortWriteBufferCheck =
- mozilla::MakeUnique<char[]>(mShortWriteOriginalAmount);
- memcpy(mShortWriteBufferCheck.get(), data, mShortWriteOriginalAmount);
- }
- void CheckShortWrittenBuffer(const unsigned char* data, int32_t amount) {
- COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
- if (!mShortWriteBufferCheck) return;
- MOZ_ASSERT(amount >= mShortWriteOriginalAmount,
- "unexpected amount length after short write");
- MOZ_ASSERT(
- !memcmp(mShortWriteBufferCheck.get(), data, mShortWriteOriginalAmount),
- "unexpected buffer content after short write");
- mShortWriteBufferCheck = nullptr;
+ MOZ_ASSERT(mMaybeShortWrite.isSome());
+ if (mMaybeShortWrite.isNothing()) {
+ return nullptr;
+ }
+ MOZ_ASSERT(amount > 0);
+ if (amount <= 0) {
+ return nullptr;
+ }
+ if (static_cast<size_t>(amount) < mMaybeShortWrite->Length()) {
+ return nullptr;
+ }
+ if (mozilla::Span(*mMaybeShortWrite) !=
+ mozilla::Span(reinterpret_cast<uint8_t*>(const_cast<void*>(data)),
+ mMaybeShortWrite->Length())) {
+ return nullptr;
+ }
+ // Return a pointer to the byte that has yet to be written.
+ return static_cast<const uint8_t*>(data) + mMaybeShortWrite->Length() - 1;
}
-#endif
nsresult SetResumptionTokenFromExternalCache(PRFileDesc* fd);
@@ -314,23 +316,10 @@ class NSSSocketControl final : public CommonSocketControl {
bool mNotedTimeUntilReady;
EchExtensionStatus mEchExtensionStatus; // Currently only used for telemetry.
- // True when SSL layer has indicated an "SSL short write", i.e. need
- // to call on send one or more times to push all pending data to write.
- bool mIsShortWritePending;
-
- // These are only valid if mIsShortWritePending is true.
- //
- // Value of the last byte pending from the SSL short write that needs
- // to be passed to subsequent calls to send to perform the flush.
- unsigned char mShortWritePendingByte;
-
- // Original amount of data the upper layer has requested to write to
- // return after the successful flush.
- int32_t mShortWriteOriginalAmount;
-
-#ifdef DEBUG
- mozilla::UniquePtr<char[]> mShortWriteBufferCheck;
-#endif
+ // If libssl has indicated a "short write", the layer above PSM must retry
+ // sending the exact same data. If this is Some, all but the last byte has
+ // already been written to libssl.
+ mozilla::Maybe<nsTArray<uint8_t>> mMaybeShortWrite;
// mKEA* are used in false start and http/2 detetermination
// Values are from nsITLSSocketControl
diff --git a/security/manager/ssl/nsNSSIOLayer.cpp b/security/manager/ssl/nsNSSIOLayer.cpp
index b24a5b21513..c51a1456d50 100644
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -821,7 +821,9 @@ static int32_t PSMRecv(PRFileDesc* fd, void* buf, int32_t amount, int flags,
static int32_t PSMSend(PRFileDesc* fd, const void* buf, int32_t amount,
int flags, PRIntervalTime timeout) {
NSSSocketControl* socketInfo = getSocketInfoIfRunning(fd, writing);
- if (!socketInfo) return -1;
+ if (!socketInfo) {
+ return -1;
+ }
if (flags != 0) {
PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
@@ -833,15 +835,13 @@ static int32_t PSMSend(PRFileDesc* fd, const void* buf, int32_t amount,
#endif
if (socketInfo->IsShortWritePending() && amount > 0) {
- // We got "SSL short write" last time, try to flush the pending byte.
-#ifdef DEBUG
- socketInfo->CheckShortWrittenBuffer(static_cast<const unsigned char*>(buf),
- amount);
-#endif
-
- buf = socketInfo->GetShortWritePendingByteRef();
+ // We got a "short write" last time, try to flush the pending byte.
+ buf = socketInfo->CheckShortWriteBuffer(buf, amount);
+ if (!buf) {
+ PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
+ return -1;
+ }
amount = 1;
-
MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
("[%p] pushing 1 byte after SSL short write", fd));
}
@@ -862,6 +862,10 @@ static int32_t PSMSend(PRFileDesc* fd, const void* buf, int32_t amount,
// in the previous call. We then return the total number of bytes written,
// which is the number that caused the short write plus the additional byte
// we just wrote out.
+ // Note that the layer above *must* retry sending the exact same data. To do
+ // otherwise is a violation of NSS's expectations and is an error.
+ // Because early data can be truncated by libssl, only perform this
+ // optimization after the handshake has completed.
// The 16384 value is based on libssl's maximum buffer size:
// MAX_FRAGMENT_LENGTH - 1
@@ -869,12 +873,11 @@ static int32_t PSMSend(PRFileDesc* fd, const void* buf, int32_t amount,
// It's in a private header, though, filed bug 1394822 to expose it.
static const int32_t kShortWrite16k = 16383;
- if ((amount > 1 && bytesWritten == (amount - 1)) ||
- (amount > kShortWrite16k && bytesWritten == kShortWrite16k)) {
- // This is indication of an "SSL short write", block to force retry.
- socketInfo->SetShortWritePending(
- bytesWritten + 1, // The amount to return after the flush
- *(static_cast<const unsigned char*>(buf) + bytesWritten));
+ if (socketInfo->IsHandshakeCompleted() &&
+ ((amount > 1 && bytesWritten == (amount - 1)) ||
+ (amount > kShortWrite16k && bytesWritten == kShortWrite16k))) {
+ // This indicates a "short write", so block to force a retry.
+ socketInfo->SetShortWritePending(buf, bytesWritten + 1);
MOZ_LOG(
gPIPNSSLog, LogLevel::Verbose,
@@ -883,24 +886,17 @@ static int32_t PSMSend(PRFileDesc* fd, const void* buf, int32_t amount,
bytesWritten = -1;
PR_SetError(PR_WOULD_BLOCK_ERROR, 0);
-
-#ifdef DEBUG
- socketInfo->RememberShortWrittenBuffer(
- static_cast<const unsigned char*>(buf));
-#endif
-
} else if (socketInfo->IsShortWritePending() && bytesWritten == 1) {
- // We have now flushed all pending data in the SSL socket
- // after the indicated short write. Tell the upper layer
- // it has sent all its data now.
+ // We have now flushed all pending data in the SSL socket after the
+ // indicated short write. Tell the upper layer it has sent all the data
+ // from that write now.
MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
("[%p] finished SSL short write", fd));
-
bytesWritten = socketInfo->ResetShortWritePending();
}
MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
- ("[%p] wrote %d bytes\n", fd, bytesWritten));
+ ("[%p] wrote %d bytes", fd, bytesWritten));
return checkHandshake(bytesWritten, false, fd, socketInfo);
}
Loading diff…
References
On This Page