Firefox · DOM
CVE-2026-109745
Logic Error in DOM
Overview
Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifdom/webserial/Win32SerialOverlappedIO.cpp |
modified | |
Win32SerialOverlappedIOdom/webserial/Win32SerialOverlappedIO.h |
modified | |
Win32SerialParityCheckStreamdom/webserial/Win32SerialParityCheckStream.h |
modified | |
ifdom/webserial/Win32SerialPlatformService.cpp |
modified | |
whiledom/webserial/Win32SerialPlatformService.cpp |
modified |
Files Changed
dom/webserial/Win32SerialOverlappedIO.cppdom/webserial/Win32SerialOverlappedIO.hdom/webserial/Win32SerialParityCheckStream.cppdom/webserial/Win32SerialParityCheckStream.hdom/webserial/Win32SerialPlatformService.cppdom/webserial/moz.build
Patch
diff --git a/dom/webserial/Win32SerialOverlappedIO.cpp b/dom/webserial/Win32SerialOverlappedIO.cpp
new file mode 100644
index 00000000000..eb868dc01e3
--- /dev/null
+++ b/dom/webserial/Win32SerialOverlappedIO.cpp
@@ -0,0 +1,98 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "Win32SerialOverlappedIO.h"
+
+// ntddser.h depends on windows.h and winioctl.h, so disable clang-format to
+// keep the include order.
+
+// clang-format off
+#include <windows.h>
+#include <winioctl.h>
+#include <ntddser.h>
+// clang-format on
+
+namespace mozilla::dom {
+
+bool Win32SerialOverlappedIO::Init() {
+ mEvent = UniqueFileHandle(CreateEvent(nullptr, TRUE, FALSE, nullptr));
+ if (!mEvent) {
+ return false;
+ }
+ mOverlapped = {};
+ // Setting the low-order bit of hEvent prevents the I/O completion from
+ // being queued to the IOCP. This is necessary because PlatformPipeReader
+ // may have registered a DuplicateHandle of this port with the IOCP, and
+ // completions from our local OVERLAPPED would corrupt the IOCP handler
+ // lookup. GetOverlappedResult still works via the event.
+ HANDLE rawEvent = mEvent.get();
+ mOverlapped.hEvent =
+ reinterpret_cast<HANDLE>(reinterpret_cast<uintptr_t>(rawEvent) | 1);
+ return true;
+}
+
+void Win32SerialOverlappedIO::Reset() { ResetEvent(mEvent.get()); }
+
+bool Win32SerialOverlappedIO::Wait(HANDLE aHandle, DWORD* aBytesTransferred) {
+ return GetOverlappedResult(aHandle, &mOverlapped, aBytesTransferred,
+ /* bWait */ TRUE);
+}
+
+/* static */
+bool Win32SerialOverlappedIO::SyncCheckParity(HANDLE aHandle) {
+ // This is what ClearCommError() does internally.
+ SERIAL_STATUS status = {};
+ return !(Win32SerialOverlappedIO::SyncDeviceIoControl(
+ aHandle, IOCTL_SERIAL_GET_COMMSTATUS, nullptr, 0, &status,
+ sizeof(status)) &&
+ (status.Errors & SERIAL_ERROR_PARITY));
+}
+
+/* static */
+bool Win32SerialOverlappedIO::SyncPurge(HANDLE aHandle, ULONG aPurgeFlags) {
+ ULONG purgeFlags = aPurgeFlags;
+ return Win32SerialOverlappedIO::SyncDeviceIoControl(
+ aHandle, IOCTL_SERIAL_PURGE, &purgeFlags, sizeof(purgeFlags), nullptr, 0);
+}
+
+/* static */
+bool Win32SerialOverlappedIO::SyncSetSignal(HANDLE aHandle,
+ DWORD aIoControlCode) {
+ return Win32SerialOverlappedIO::SyncDeviceIoControl(aHandle, aIoControlCode,
+ nullptr, 0, nullptr, 0);
+}
+
+/* static */
+bool Win32SerialOverlappedIO::SyncGetSignals(HANDLE aHandle, ULONG& aSignals) {
+ return Win32SerialOverlappedIO::SyncDeviceIoControl(
+ aHandle, IOCTL_SERIAL_GET_MODEMSTATUS, nullptr, 0, &aSignals,
+ sizeof(aSignals));
+}
+
+/* static */
+bool Win32SerialOverlappedIO::SyncDeviceIoControl(
+ HANDLE aHandle, DWORD aIoControlCode, const void* aInBuffer,
+ DWORD aInBufferSize, void* aOutBuffer, DWORD aOutBufferSize,
+ DWORD* aBytesReturned) {
+ Win32SerialOverlappedIO io;
+ if (!io.Init()) {
+ return false;
+ }
+ if (!DeviceIoControl(aHandle, aIoControlCode, const_cast<void*>(aInBuffer),
+ aInBufferSize, aOutBuffer, aOutBufferSize,
+ /* lpBytesReturned */ nullptr, io.Get()) &&
+ GetLastError() != ERROR_IO_PENDING) {
+ return false;
+ }
+ DWORD bytes = 0;
+ if (!io.Wait(aHandle, &bytes)) {
+ return false;
+ }
+ if (aBytesReturned) {
+ *aBytesReturned = bytes;
+ }
+ return true;
+}
+
+} // namespace mozilla::dom
diff --git a/dom/webserial/Win32SerialOverlappedIO.h b/dom/webserial/Win32SerialOverlappedIO.h
new file mode 100644
index 00000000000..dfeca1971c5
--- /dev/null
+++ b/dom/webserial/Win32SerialOverlappedIO.h
@@ -0,0 +1,74 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_Win32SerialOverlappedIO_h
+#define mozilla_dom_Win32SerialOverlappedIO_h
+
+#include <windows.h>
+
+#include "mozilla/UniquePtrExtensions.h"
+
+namespace mozilla::dom {
+
+// Serial ports are opened with FILE_FLAG_OVERLAPPED, and the same kernel file
+// object is shared via DuplicateHandle with a PlatformPipeReader that keeps
+// an eventless overlapped ReadFile pending on an IOCP. Issuing synchronous
+// (NULL lpOverlapped) calls on such a handle is unsafe: the Win32 wrapper waits
+// on the file object's event, which the reader's completion also signals, so
+// the wrapper can return while its IRP is still pending. Every operation on the
+// port should therefore go through this class, which supplies a private event
+// so the wait is only satisfied by our own completion.
+class Win32SerialOverlappedIO final {
+ public:
+ Win32SerialOverlappedIO() = default;
+ Win32SerialOverlappedIO(const Win32SerialOverlappedIO&) = delete;
+ Win32SerialOverlappedIO& operator=(const Win32SerialOverlappedIO&) = delete;
+
+ // Creates the event. Returns true on success, and on failure
+ // GetLastError() describes the error.
+ bool Init();
+
+ // Resets the event so the OVERLAPPED can be reused for another operation.
+ void Reset();
+
+ OVERLAPPED* Get() { return &mOverlapped; }
+
+ // Blocks until the operation issued with Get() completes. Returns true on
+ // success, and on failure GetLastError() describes the error.
+ bool Wait(HANDLE aHandle, DWORD* aBytesTransferred);
+
+ // Checks for parity errors on an overlapped handle and blocks until
+ // it completes. Returns true on success and false if there was a parity
+ // error.
+ static bool SyncCheckParity(HANDLE aHandle);
+
+ // Purges the specified buffers on an overlapped handle and blocks
+ // until it completes. Returns true on success. aPurgeFlags can be
+ // any combination of SERIAL_PURGE_RXCLEAR and SERIAL_PURGE_TXCLEAR.
+ static bool SyncPurge(HANDLE aHandle, ULONG aPurgeFlags);
+
+ // Sets or clears the passed-in signal and blocks until it completes.
+ // Returns true on success.
+ static bool SyncSetSignal(HANDLE aHandle, DWORD aIoControlCode);
+
+ // Gets the signals into aSignals and blocks until it completes.
+ // Returns true on success.
+ static bool SyncGetSignals(HANDLE aHandle, ULONG& aSignals);
+
+ private:
+ // Issues DeviceIoControl on an overlapped handle and blocks until it
+ // completes. Returns true on success, and on failure GetLastError()
+ // describes the error.
+ static bool SyncDeviceIoControl(HANDLE aHandle, DWORD aIoControlCode,
+ const void* aInBuffer, DWORD aInBufferSize,
+ void* aOutBuffer, DWORD aOutBufferSize,
+ DWORD* aBytesReturned = nullptr);
+
+ UniqueFileHandle mEvent;
+ OVERLAPPED mOverlapped = {};
+};
+
+} // namespace mozilla::dom
+
+#endif // mozilla_dom_Win32SerialOverlappedIO_h
diff --git a/dom/webserial/Win32SerialParityCheckStream.cpp b/dom/webserial/Win32SerialParityCheckStream.cpp
index 525dc62ed68..06c690ad436 100644
--- a/dom/webserial/Win32SerialParityCheckStream.cpp
+++ b/dom/webserial/Win32SerialParityCheckStream.cpp
@@ -4,9 +4,8 @@
#include "Win32SerialParityCheckStream.h"
-#include <windows.h>
-
#include "SerialLogging.h"
+#include "Win32SerialOverlappedIO.h"
#include "nsError.h"
#include "nsStreamUtils.h"
@@ -25,11 +24,10 @@ void Win32SerialParityCheckStream::CheckForParityError() {
if (MOZ_UNLIKELY(mParityErrorLatched)) {
return;
}
- DWORD errors = 0;
- if (ClearCommError(mCommHandle.get(), &errors, nullptr) &&
- (errors & CE_RXPARITY)) {
+ if (!Win32SerialOverlappedIO::SyncCheckParity(mCommHandle.get())) {
MOZ_LOG(gWebSerialLog, LogLevel::Debug,
- ("Win32SerialParityCheckStream[%p] detected CE_RXPARITY", this));
+ ("Win32SerialParityCheckStream[%p] detected SERIAL_ERROR_PARITY",
+ this));
mParityErrorLatched = true;
}
}
diff --git a/dom/webserial/Win32SerialParityCheckStream.h b/dom/webserial/Win32SerialParityCheckStream.h
index ab06ea156b4..13a8790f4ec 100644
--- a/dom/webserial/Win32SerialParityCheckStream.h
+++ b/dom/webserial/Win32SerialParityCheckStream.h
@@ -13,12 +13,13 @@ namespace mozilla::dom {
// Wraps the raw serial read stream on Windows when parity checking is enabled.
// Windows does not mark parity errors inline in the byte stream; instead the
-// driver records them and they are reported by ClearCommError(). This stream
-// delegates reads to the inner stream and, after each read, polls
-// ClearCommError() for CE_RXPARITY. On detecting a parity error it delivers the
-// bytes from the current read and then fails its next read with
-// NS_ERROR_DOM_SERIAL_PARITY_ERROR, which rides the DataPipe close channel to
-// the content process to surface a "ParityError" DOMException.
+// driver records them and they are reported by IOCTL_SERIAL_GET_COMMSTATUS
+// (the IOCTL behind ClearCommError()). This stream delegates reads to the inner
+// stream and, after each read, polls that IOCTL for SERIAL_ERROR_PARITY. On
+// detecting a parity error it delivers the bytes from the current read and
+// then fails its next read with NS_ERROR_DOM_SERIAL_PARITY_ERROR, which rides
+// the DataPipe close channel to the content process to surface a "ParityError"
+// DOMException.
class Win32SerialParityCheckStream final : public nsIAsyncInputStream {
public:
NS_DECL_THREADSAFE_ISUPPORTS
@@ -31,15 +32,15 @@ class Win32SerialParityCheckStream final : public nsIAsyncInputStream {
private:
~Win32SerialParityCheckStream() = default;
- // Polls ClearCommError() and latches a parity error if CE_RXPARITY is set.
+ // Polls the comm status and latches a parity error if one is reported.
void CheckForParityError();
nsCOMPtr<nsIAsyncInputStream> mInner;
- // A duplicate of the comm port handle, used solely for ClearCommError().
+ // A duplicate of the comm port handle, used solely for comm status polling.
UniqueFileHandle mCommHandle;
- // Set once CE_RXPARITY has been observed. After the bytes from the read that
- // observed it are delivered, the next read fails with the parity error.
+ // Set once a parity error has been observed. After the bytes from the read
+ // that observed it are delivered, the next read fails with the parity error.
bool mParityErrorLatched = false;
};
diff --git a/dom/webserial/Win32SerialPlatformService.cpp b/dom/webserial/Win32SerialPlatformService.cpp
index b90ac13fbac..e4acdb399bb 100644
--- a/dom/webserial/Win32SerialPlatformService.cpp
+++ b/dom/webserial/Win32SerialPlatformService.cpp
@@ -5,19 +5,22 @@
#include "Win32SerialPlatformService.h"
#include <cfgmgr32.h>
-// Including initguid.h needs to come before including devpkey.h, so
+// Including initguid.h needs to come before including devpkey.h, and
+// winioctl.h must come before initguid.h and before ntddser.h, so
// disable clang-format here.
// clang-format off
+#include <winioctl.h>
#include <initguid.h>
#include <devpkey.h>
+#include <ntddser.h>
// clang-format on
-#include <ntddser.h>
#include <setupapi.h>
#include "Serial.h"
#include "SerialLogging.h"
+#include "Win32SerialOverlappedIO.h"
#include "Win32SerialParityCheckStream.h"
#include "mozilla/AsyncPlatformPipes.h"
#include "mozilla/ScopeExit.h"
@@ -222,6 +225,10 @@ HANDLE Win32SerialPlatformService::FindPortHandle(const nsString& aPortId) {
return mOpenPorts.MaybeGet(aPortId).valueOr(INVALID_HANDLE_VALUE);
}
+// The synchronous Get/SetCommState and SetCommTimeouts calls below are only
+// safe because this runs from OpenImpl, before any PlatformPipeReader shares
+// the file object. Anything that runs after GetReadStreamImpl must go through
+// Win32SerialOverlappedIO instead.
nsresult Win32SerialPlatformService::ConfigurePort(
HANDLE aHandle, const IPCSerialOptions& aOptions) {
MOZ_LOG(gWebSerialLog, LogLevel::Debug,
@@ -412,7 +419,8 @@ nsresult Win32SerialPlatformService::OpenImpl(
return rv;
}
- PurgeComm(handle, PURGE_RXCLEAR | PURGE_TXCLEAR);
+ Win32SerialOverlappedIO::SyncPurge(
+ handle, SERIAL_PURGE_RXCLEAR | SERIAL_PURGE_TXCLEAR);
mOpenPorts.InsertOrUpdate(aPortId, handle);
MOZ_LOG(gWebSerialLog, LogLevel::Info,
@@ -470,24 +478,16 @@ nsresult Win32SerialPlatformService::WriteImpl(const nsString& aPortId,
const uint8_t* buffer = aData.Elements();
DWORD remaining = static_cast<DWORD>(aData.Length());
- auto event = UniqueFileHandle(CreateEvent(nullptr, TRUE, FALSE, nullptr));
- if (!event) {
+ Win32SerialOverlappedIO io;
+ if (!io.Init()) {
return NS_ERROR_FAILURE;
}
- OVERLAPPED ov = {};
- // Setting the low-order bit of hEvent prevents the I/O completion from
- // being queued to the IOCP. This is necessary because PlatformPipeReader
- // may have registered a DuplicateHandle of this port with the IOCP, and
- // completions from our local OVERLAPPED would corrupt the IOCP handler
- // lookup. GetOverlappedResult still works via the event.
- HANDLE rawEvent = event.get();
- ov.hEvent =
- reinterpret_cast<HANDLE>(reinterpret_cast<uintptr_t>(rawEvent) | 1);
while (remaining > 0) {
- ResetEvent(event.get());
+ io.Reset();
- if (!WriteFile(handle, buffer + totalWritten, remaining, nullptr, &ov)) {
+ if (!WriteFile(handle, buffer + totalWritten, remaining, nullptr,
+ io.Get())) {
DWORD error = GetLastError();
if (error != ERROR_IO_PENDING) {
MOZ_LOG(
@@ -499,7 +499,7 @@ nsresult Win32SerialPlatformService::WriteImpl(const nsString& aPortId,
}
}
DWORD bytesWritten = 0;
- if (!GetOverlappedResult(handle, &ov, &bytesWritten, TRUE)) {
+ if (!io.Wait(handle, &bytesWritten)) {
MOZ_LOG(gWebSerialLog, LogLevel::Error,
("Win32SerialPlatformService[%p]::Write GetOverlappedResult "
"failed for port '%s': 0x%08lx",
@@ -549,6 +549,9 @@ nsresult Win32SerialPlatformService::DrainImpl(const nsString& aPortId) {
"port '%s'",
this, NS_ConvertUTF16toUTF8(aPortId).get()));
+ // Unlike the DeviceIoControl-based comm APIs, FlushFileBuffers is safe on a
+ // shared overlapped handle: NtFlushBuffersFile waits on an event it
+ // allocates itself rather than on the file object's event.
if (!FlushFileBuffers(handle)) {
DWORD error = GetLastError();
MOZ_LOG(
@@ -577,18 +580,18 @@ nsresult Win32SerialPlatformService::FlushImpl(const nsString& aPortId,
return NS_ERROR_NOT_AVAILABLE;
}
- DWORD flags = aReceive ? PURGE_RXCLEAR : PURGE_TXCLEAR;
+ ULONG flags = aReceive ? SERIAL_PURGE_RXCLEAR : SERIAL_PURGE_TXCLEAR;
MOZ_LOG(gWebSerialLog, LogLevel::Debug,
("Win32SerialPlatformService[%p]::Flush discarding %s buffers "
"for port '%s'",
this, aReceive ? "receive" : "transmit",
NS_ConvertUTF16toUTF8(aPortId).get()));
- if (!PurgeComm(handle, flags)) {
+ if (!Win32SerialOverlappedIO::SyncPurge(handle, flags)) {
DWORD error = GetLastError();
MOZ_LOG(gWebSerialLog, LogLevel::Error,
- ("Win32SerialPlatformService[%p]::Flush PurgeComm failed for port "
- "'%s': error=%lu",
+ ("Win32SerialPlatformService[%p]::Flush IOCTL_SERIAL_PURGE failed "
+ "for port '%s': error=%lu",
this, NS_ConvertUTF16toUTF8(aPortId).get(), error));
return NS_ERROR_FAILURE;
}
@@ -626,41 +629,42 @@ nsresult Win32SerialPlatformService::SetSignalsImpl(
return NS_ERROR_NOT_AVAILABLE;
}
- if (aSignals.dataTerminalReady().isSome()) {
- if (!EscapeCommFunction(
- handle, aSignals.dataTerminalReady().value() ? SETDTR : CLRDTR)) {
+ auto setSignal = [&](const char* aName, DWORD aIoControlCode) -> nsresult {
+ if (!Win32SerialOverlappedIO::SyncSetSignal(handle, aIoControlCode)) {
DWORD error = GetLastError();
- MOZ_LOG(
- gWebSerialLog, LogLevel::Error,
- ("Win32SerialPlatformService[%p]::SetSignals EscapeCommFunction DTR "
- "failed for port '%s': 0x%08lx",
- this, NS_ConvertUTF16toUTF8(aPortId).get(), error));
+ MOZ_LOG(gWebSerialLog, LogLevel::Error,
+ ("Win32SerialPlatformService[%p]::SetSignals %s failed for port "
+ "'%s': 0x%08lx",
+ this, aName, NS_ConvertUTF16toUTF8(aPortId).get(), error));
return NS_ERROR_FAILURE;
... (file diff truncated)
Loading diff…
References
On This Page