Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionClickjacking issue in the Widget component
ComponentWidget
Bug ClassLogic Error
Tracker2036097
Fix commitdc7f8a547186 (firefox) +56/-355
CISA KEVNot listed
Creditedhafidzaulia28
Disclosed2026-08-18

Changed Functions

FunctionChangeNotes
if
widget/cocoa/nsFilePicker.mm
modified

Files Changed

  • modules/libpref/init/StaticPrefList.yaml
  • modules/libpref/init/all.js
  • widget/cocoa/nsFilePicker.h
  • widget/cocoa/nsFilePicker.mm
  • widget/gtk/nsFilePicker.cpp
  • widget/nsBaseFilePicker.cpp
  • widget/nsBaseFilePicker.h
  • widget/tests/gtest/TestFilePickerInputProtection.cpp
  • widget/tests/gtest/moz.build
  • widget/windows/filedialog/PWinFileDialog.ipdl
  • widget/windows/filedialog/WinFileDialogChild.cpp
  • widget/windows/filedialog/WinFileDialogChild.h
  • widget/windows/filedialog/WinFileDialogCommands.cpp
  • widget/windows/filedialog/WinFileDialogCommands.h
  • widget/windows/filedialog/WinFileDialogParent.cpp
  • widget/windows/filedialog/WinFileDialogParent.h
  • widget/windows/nsFilePicker.cpp
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
index dd122a67544..4843edb3fbe 100644
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -18792,16 +18792,6 @@
   value: false
   mirror: always
 
-# How long (in milliseconds) security-sensitive notifications delay before
-# their buttons become enabled, to guard against clickjacking. Native file
-# pickers reuse this value to ignore a confirmation that arrives too soon after
-# the picker is shown, keeping the picker open instead of accepting it. Set to
-# 0 to disable.
-- name: security.notification_enable_delay
-  type: RelaxedAtomicUint32
-  value: 500
-  mirror: always
-
 # Whether or not to import and trust third party root certificates from the OS.
 - name: security.enterprise_roots.enabled
   type: RelaxedAtomicBool
diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
index dadb6a117b4..b1f9b54bf54 100644
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1735,6 +1735,7 @@ pref("font.blacklist.underline_offset", "FangSong,Gulim,GulimChe,MingLiU,MingLiU
 
 // security-sensitive dialogs should delay button enabling. In milliseconds.
 pref("security.dialog_enable_delay", 1000);
+pref("security.notification_enable_delay", 500);
 
 #ifdef NIGHTLY_BUILD
   // Disallow web documents loaded with the SystemPrincipal
diff --git a/widget/cocoa/nsFilePicker.h b/widget/cocoa/nsFilePicker.h
index 288a8c7c607..1b592b1e5bf 100644
--- a/widget/cocoa/nsFilePicker.h
+++ b/widget/cocoa/nsFilePicker.h
@@ -44,11 +44,6 @@ class nsFilePicker final : public nsBaseFilePicker {
    */
   NSArray* GetFilterList();
 
-  // Exposed for the panel delegate (MOZFilePickerInputProtector), which lives
-  // outside the class, so it can veto confirmations that arrive before the
-  // input-protection window elapses.
-  using nsBaseFilePicker::IsPickerInputProtected;
-
  protected:
   virtual ~nsFilePicker();
 
diff --git a/widget/cocoa/nsFilePicker.mm b/widget/cocoa/nsFilePicker.mm
index 02287820c3b..fdf05b6ffd5 100644
--- a/widget/cocoa/nsFilePicker.mm
+++ b/widget/cocoa/nsFilePicker.mm
@@ -49,42 +49,6 @@ const char kShowHiddenFilesPref[] = "filepicker.showHiddenFiles";
 - (void)menuChangedItem:(NSNotification*)aSender;
 @end
 
-// Panel delegate that ignores confirmations that arrive before the file
-// picker's input-protection time range has passed. Returning NO from
-// panel:validateURL:error: to keep the panel open is the documented purpose
-// of the method; leaving the error nil so no alert is shown is undocumented,
-// but observed to hold (including for the out-of-process panel). If a future
-// macOS changes this, the worst case is a stray alert or the check quietly
-// doing nothing, and it can be turned off with
-// security.notification_enable_delay.
-@interface MOZFilePickerInputProtector : NSObject <NSOpenSavePanelDelegate> {
-  RefPtr<nsFilePicker> mFilePicker;
-}
-- (id)initWithFilePicker:(nsFilePicker*)aFilePicker;
-@end
-
-@implementation MOZFilePickerInputProtector
-- (id)initWithFilePicker:(nsFilePicker*)aFilePicker {
-  if ((self = [super init])) {
-    mFilePicker = aFilePicker;
-  }
-  return self;
-}
-
-- (BOOL)panel:(id)sender validateURL:(NSURL*)url error:(NSError**)outError {
-  // url is intentionally unused: the file we return is read from the panel
-  // when it finally closes, so an ignored early confirmation can't pin a
-  // stale selection.
-  if (mFilePicker && mFilePicker->IsPickerInputProtected()) {
-    if (outError) {
-      *outError = nil;
-    }
-    return NO;
-  }
-  return YES;
-}
-@end
-
 NS_IMPL_ISUPPORTS(nsFilePicker, nsIFilePicker)
 
 static void SetShowHiddenFileState(NSSavePanel* panel) {
@@ -266,25 +230,10 @@ void nsFilePicker::BeginPanelAsync(NSSavePanel* aPanel,
     parentWindow =
         static_cast<NSWindow*>(mParentWidget->GetNativeData(NS_NATIVE_WINDOW));
   }
-
-  // Attach a delegate to ignore confirmations that arrive before the
-  // input-protection time range has passed. The panel does not retain its
-  // delegate, so the completion handler releases it.
-  MOZFilePickerInputProtector* protector =
-      [[MOZFilePickerInputProtector alloc] initWithFilePicker:this];
-  [aPanel setDelegate:protector];
-
-  void (^handler)(NSModalResponse) = ^(NSModalResponse result) {
-    aHandler(result);
-    [aPanel setDelegate:nil];
-    [protector release];
-  };
-
-  RecordLastShownTime();
   if (parentWindow) {
-    [aPanel beginSheetModalForWindow:parentWindow completionHandler:handler];
+    [aPanel beginSheetModalForWindow:parentWindow completionHandler:aHandler];
   } else {
-    [aPanel beginWithCompletionHandler:handler];
+    [aPanel beginWithCompletionHandler:aHandler];
   }
 }
 
diff --git a/widget/gtk/nsFilePicker.cpp b/widget/gtk/nsFilePicker.cpp
index f473b2420ad..56d33ae6cba 100644
--- a/widget/gtk/nsFilePicker.cpp
+++ b/widget/gtk/nsFilePicker.cpp
@@ -796,7 +796,6 @@ void nsFilePicker::OpenNonPortal() {
                    this);
   g_signal_connect(file_chooser, "destroy", G_CALLBACK(OnNonPortalDestroy),
                    this);
-  RecordLastShownTime();
   gtk_widget_show(GTK_WIDGET(file_chooser));
 }
 
@@ -873,15 +872,6 @@ bool nsFilePicker::WarnForNonReadableFile() {
 }
 
 void nsFilePicker::DoneNonPortal(GtkWidget* file_chooser, gint response) {
-  // Ignore a confirmation that arrives before the input-protection time range
-  // has passed, leaving the dialog open. (GTK emits GTK_RESPONSE_ACCEPT on a
-  // double-click.)
-  if ((response == GTK_RESPONSE_OK || response == GTK_RESPONSE_ACCEPT ||
-       response == kFilePickerAccept) &&
-      IsPickerInputProtected()) {
-    return;
-  }
-
   mFileChooser = nullptr;
 
   nsIFilePicker::ResultCode result;
diff --git a/widget/nsBaseFilePicker.cpp b/widget/nsBaseFilePicker.cpp
index 060a3ed187c..1be546a755d 100644
--- a/widget/nsBaseFilePicker.cpp
+++ b/widget/nsBaseFilePicker.cpp
@@ -6,9 +6,7 @@
 #include "nsBaseFilePicker.h"
 
 #include "WidgetUtils.h"
-#include "mozilla/BasePrincipal.h"
 #include "mozilla/Components.h"
-#include "mozilla/StaticPrefs_security.h"
 #include "mozilla/StaticPrefs_widget.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/CanonicalBrowsingContext.h"
@@ -16,8 +14,16 @@
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/Promise.h"
-#include "mozilla/dom/WindowGlobalParent.h"
 #include "nsArrayEnumerator.h"
+#include "nsCOMArray.h"
+#include "nsCOMPtr.h"
+#include "nsContentUtils.h"
+#include "nsEnumeratorUtils.h"
+#include "nsIFile.h"
+#include "nsIInterfaceRequestorUtils.h"
+#include "nsIStringBundle.h"
+#include "nsIWidget.h"
+#include "nsPIDOMWindow.h"
 #include "nsSimpleEnumerator.h"
 #include "nsString.h"
 #include "nsThreadUtils.h"
@@ -374,49 +380,6 @@ bool nsBaseFilePicker::MaybeBlockFilePicker(
   return true;
 }
 
-// static
-bool nsBaseFilePicker::IsWithinInputProtectionTimeRange(
-    mozilla::TimeStamp aShowTime, mozilla::TimeStamp aNow,
-    uint32_t aProtectionMs) {
-  if (!aProtectionMs || aShowTime.IsNull()) {
-    return false;
-  }
-  // TimeStamp only moves forward, so the current time should never be earlier
-  // than the shown time. We do not expect this to happen; if it ever does we
-  // want to find out and decide how to handle it, so assert on debug and
-  // Nightly builds. On other builds, fall back to "not protected" so we never
-  // block the user from confirming.
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/widget/tests/gtest/TestFilePickerInputProtection.cpp b/widget/tests/gtest/TestFilePickerInputProtection.cpp
deleted file mode 100644
index 50bf4c5f940..00000000000
--- a/widget/tests/gtest/TestFilePickerInputProtection.cpp
+++ /dev/null
@@ -1,55 +0,0 @@
-/* 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 "gtest/gtest.h"
-#include "mozilla/TimeStamp.h"
-#include "nsBaseFilePicker.h"
-
-using mozilla::TimeDuration;
-using mozilla::TimeStamp;
-
-// Tests the shared timing check used by every platform's file picker to
-// ignore confirmations that arrive before the input-protection time range has
-// passed.
-
-TEST(FilePickerInputProtection, WithinTimeRangeIsProtected)
-{
-  TimeStamp show = TimeStamp::Now();
-  TimeStamp now = show + TimeDuration::FromMilliseconds(100);
-  EXPECT_TRUE(
-      nsBaseFilePicker::IsWithinInputProtectionTimeRange(show, now, 500));
-}
-
-TEST(FilePickerInputProtection, PastTimeRangeIsNotProtected)
-{
-  TimeStamp show = TimeStamp::Now();
-  TimeStamp now = show + TimeDuration::FromMilliseconds(600);
-  EXPECT_FALSE(
-      nsBaseFilePicker::IsWithinInputProtectionTimeRange(show, now, 500));
-}
-
-TEST(FilePickerInputProtection, BoundaryIsNotProtected)
-{
-  // The check uses "less than", so a confirmation exactly at the end of the
-  // time range is accepted.
-  TimeStamp show = TimeStamp::Now();
-  TimeStamp now = show + TimeDuration::FromMilliseconds(500);
-  EXPECT_FALSE(
-      nsBaseFilePicker::IsWithinInputProtectionTimeRange(show, now, 500));
-}
-
-TEST(FilePickerInputProtection, ZeroProtectionDisablesTimeRange)
-{
-  TimeStamp show = TimeStamp::Now();
-  TimeStamp now = show + TimeDuration::FromMilliseconds(1);
-  EXPECT_FALSE(
-      nsBaseFilePicker::IsWithinInputProtectionTimeRange(show, now, 0));
-}
-
-TEST(FilePickerInputProtection, NullShowTimeDisablesTimeRange)
-{
-  TimeStamp now = TimeStamp::Now();
-  EXPECT_FALSE(nsBaseFilePicker::IsWithinInputProtectionTimeRange(TimeStamp(),
-                                                                  now, 500));
-}
diff --git a/widget/tests/gtest/moz.build b/widget/tests/gtest/moz.build
index 005ce05e110..4e73b6d15fe 100644
--- a/widget/tests/gtest/moz.build
+++ b/widget/tests/gtest/moz.build
@@ -3,7 +3,6 @@
 # file, you can obtain one at http://mozilla.org/MPL/2.0/.
 
 UNIFIED_SOURCES = [
-    "TestFilePickerInputProtection.cpp",
     "TestGfxInfo.cpp",
     "TestTimeConverter.cpp",
     "TestTouchResampler.cpp",
Loading diff…