Moderate firefox Logic Error 🔧 Commit mapped

Overview

Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionSandbox escape due to uninitialized memory in the DOM: Web Serial component
ComponentDOM
Bug ClassLogic Error
Tracker2061784
Fix commitddfefa260bf0 (firefox) +244/-69
CISA KEVNot listed
CreditedMozilla
Disclosed2026-10-13

Changed Functions

FunctionChangeNotes
if
dom/webserial/Win32SerialOverlappedIO.cpp
modified
Win32SerialOverlappedIO
dom/webserial/Win32SerialOverlappedIO.h
modified
Win32SerialParityCheckStream
dom/webserial/Win32SerialParityCheckStream.h
modified
if
dom/webserial/Win32SerialPlatformService.cpp
modified
while
dom/webserial/Win32SerialPlatformService.cpp
modified

Files Changed

  • dom/webserial/Win32SerialOverlappedIO.cpp
  • dom/webserial/Win32SerialOverlappedIO.h
  • dom/webserial/Win32SerialParityCheckStream.cpp
  • dom/webserial/Win32SerialParityCheckStream.h
  • dom/webserial/Win32SerialPlatformService.cpp
  • dom/webserial/moz.build
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…