From 0d51adc2173e42cadc13984e6a4b4ca56e31bb2f Mon Sep 17 00:00:00 2001 From: Martok Date: Sat, 10 Feb 2024 16:29:23 +0100 Subject: [PATCH 1/6] Issue #2453 - Follow-Up: Use hard-coded minimum timer interval in workers. From the spec: "If nesting level is greater than 5, and timeout is less than 4, then set timeout to 4.", therefore not having it user-configurable doesn't really break anything. --- dom/workers/WorkerPrefs.h | 1 - dom/workers/WorkerPrivate.cpp | 16 ++++------------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/dom/workers/WorkerPrefs.h b/dom/workers/WorkerPrefs.h index a8a1e61f4f..415435cf06 100644 --- a/dom/workers/WorkerPrefs.h +++ b/dom/workers/WorkerPrefs.h @@ -26,7 +26,6 @@ WORKER_SIMPLE_PREF("browser.dom.window.dump.enabled", DumpEnabled, DUMP) WORKER_SIMPLE_PREF("canvas.imagebitmap_extensions.enabled", ImageBitmapExtensionsEnabled, IMAGEBITMAP_EXTENSIONS_ENABLED) WORKER_SIMPLE_PREF("dom.caches.enabled", DOMCachesEnabled, DOM_CACHES) WORKER_SIMPLE_PREF("dom.caches.testing.enabled", DOMCachesTestingEnabled, DOM_CACHES_TESTING) -WORKER_SIMPLE_PREF("dom.min_timeout_value", DOMMinTimeoutValue, DOM_MIN_TIMEOUT_VALUE) WORKER_SIMPLE_PREF("dom.performance.enable_user_timing_logging", PerformanceLoggingEnabled, PERFORMANCE_LOGGING_ENABLED) WORKER_SIMPLE_PREF("dom.webnotifications.enabled", DOMWorkerNotificationEnabled, DOM_WORKERNOTIFICATION) WORKER_SIMPLE_PREF("dom.webnotifications.serviceworker.enabled", DOMServiceWorkerNotificationEnabled, DOM_SERVICEWORKERNOTIFICATION) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 9c52c789cc..85bcc2ffef 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -171,6 +171,8 @@ const nsIID kDEBUGWorkerEventTargetIID = { // The number of nested timeouts before we start clamping. HTML says 5. const uint32_t kClampTimeoutNestingLevel = 5u; +// The minimum interval we clamp timers in workers to. HTML says 4ms. +const uint32_t kClampTimeoutInterval = 4u; template class AutoPtrComparator @@ -1997,20 +1999,10 @@ struct WorkerPrivate::TimeoutInfo void CalculateTargetTime(JSContext* aCx) { auto target = mInterval; - int32_t minTimeoutValue; - - // We're on a worker thread; go through WorkerPrivate for the pref. - WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(aCx); - if (workerPrivate) { - minTimeoutValue = workerPrivate->DOMMinTimeoutValue(); - } else { - // fall back to default 4 ms - minTimeoutValue = 4; - } - + // Clamp timeout for workers, except chrome workers if (mNestingLevel >= kClampTimeoutNestingLevel && !mOnChromeWorker) { - target = TimeDuration::Max(mInterval,TimeDuration::FromMilliseconds(minTimeoutValue)); + target = TimeDuration::Max(mInterval, TimeDuration::FromMilliseconds(kClampTimeoutInterval)); } mTargetTime = TimeStamp::Now() + target; } From 6e1c5855398609a6e7262a6931bb80e89bd8131b Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Thu, 22 Feb 2024 09:07:04 +0800 Subject: [PATCH 2/6] No issue - Remove leftover doorhanger code from the developer tools Partially based on reverting https://bugzilla.mozilla.org/show_bug.cgi?id=1078539 --- devtools/client/framework/toolbox.js | 2 - .../responsivedesign/responsivedesign.jsm | 1 - devtools/client/shared/doorhanger.js | 137 ------------------ devtools/client/shared/moz.build | 1 - devtools/client/webconsole/hudservice.js | 1 - 5 files changed, 142 deletions(-) delete mode 100644 devtools/client/shared/doorhanger.js diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 4ec8647eb5..9b9fa66330 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -46,8 +46,6 @@ loader.lazyRequireGetter(this, "InspectorFront", "devtools/shared/fronts/inspector", true); loader.lazyRequireGetter(this, "flags", "devtools/shared/flags"); -loader.lazyRequireGetter(this, "showDoorhanger", - "devtools/client/shared/doorhanger", true); loader.lazyRequireGetter(this, "createPerformanceFront", "devtools/shared/fronts/performance", true); loader.lazyRequireGetter(this, "system", diff --git a/devtools/client/responsivedesign/responsivedesign.jsm b/devtools/client/responsivedesign/responsivedesign.jsm index 116ee3fb8e..0b41c97591 100644 --- a/devtools/client/responsivedesign/responsivedesign.jsm +++ b/devtools/client/responsivedesign/responsivedesign.jsm @@ -8,7 +8,6 @@ const Cu = Components.utils; var {loader, require} = Cu.import("resource://devtools/shared/Loader.jsm", {}); var Telemetry = require("devtools/client/shared/telemetry"); -var {showDoorhanger} = require("devtools/client/shared/doorhanger"); var {TouchEventSimulator} = require("devtools/shared/touch/simulator"); var {Task} = require("devtools/shared/task"); var promise = require("promise"); diff --git a/devtools/client/shared/doorhanger.js b/devtools/client/shared/doorhanger.js deleted file mode 100644 index 4c6d6f8cb7..0000000000 --- a/devtools/client/shared/doorhanger.js +++ /dev/null @@ -1,137 +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/. */ - -"use strict"; - -const { Ci, Cc } = require("chrome"); -const Services = require("Services"); -const { DOMHelpers } = require("resource://devtools/client/shared/DOMHelpers.jsm"); -const { Task } = require("devtools/shared/task"); -const defer = require("devtools/shared/defer"); -const { getMostRecentBrowserWindow } = require("sdk/window/utils"); - -const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"; -const LOCALE = Cc["@mozilla.org/chrome/chrome-registry;1"] - .getService(Ci.nsIXULChromeRegistry) - .getSelectedLocale("global"); - -var TYPES = { - // We don't support any doorhanger types at the moment. - // This is vestigial from the FF dev edition promo. -}; - -var panelAttrs = { - orient: "vertical", - hidden: "false", - consumeoutsideclicks: "true", - noautofocus: "true", - align: "start", - role: "alert" -}; - -/** - * Helper to call a doorhanger, defined in `TYPES`, with defined conditions, - * success handlers and loads its own XUL in a frame. Takes an object with - * several properties: - * - * @param {XULWindow} window - * The window that should house the doorhanger. - * @param {String} type - * The type of doorhanger to be displayed is, using the `TYPES` - * definition. - * @param {String} selector - * The selector that the doorhanger should be appended to within - * `window`. Defaults to a XUL Document's `window` element. - */ -exports.showDoorhanger = Task.async(function* ({ window, type, anchor }) { - let { predicate, success, url, action } = TYPES[type]; - // Abort if predicate fails - if (!predicate()) { - return; - } - - // Call success function to set preferences/cleanup immediately, - // so if triggered multiple times, only happens once (Windows/Linux) - success(); - - // Wait 200ms to prevent flickering where the popup is displayed - // before the underlying window (Windows 7, 64bit) - yield wait(200); - - let document = window.document; - - let panel = document.createElementNS(XULNS, "panel"); - let frame = document.createElementNS(XULNS, "iframe"); - let parentEl = document.querySelector("window"); - - frame.setAttribute("src", url); - let close = () => parentEl.removeChild(panel); - - setDoorhangerStyle(panel, frame); - - panel.appendChild(frame); - parentEl.appendChild(panel); - - yield onFrameLoad(frame); - - panel.openPopup(anchor); - - let closeBtn = frame.contentDocument.querySelector("#close"); - if (closeBtn) { - closeBtn.addEventListener("click", close); - } - - let goBtn = frame.contentDocument.querySelector("#go"); - if (goBtn) { - goBtn.addEventListener("click", () => { - if (action) { - action(); - } - close(); - }); - } -}); - -function setDoorhangerStyle(panel, frame) { - Object.keys(panelAttrs).forEach(prop => { - return panel.setAttribute(prop, panelAttrs[prop]); - }); - panel.style.margin = "20px"; - panel.style.borderRadius = "5px"; - panel.style.border = "none"; - panel.style.MozAppearance = "none"; - panel.style.backgroundColor = "transparent"; - - frame.style.borderRadius = "5px"; - frame.setAttribute("flex", "1"); - frame.setAttribute("width", "450"); - frame.setAttribute("height", "179"); -} - -function onFrameLoad(frame) { - let { resolve, promise } = defer(); - - if (frame.contentWindow) { - let domHelper = new DOMHelpers(frame.contentWindow); - domHelper.onceDOMReady(resolve); - } else { - let callback = () => { - frame.removeEventListener("DOMContentLoaded", callback); - resolve(); - }; - frame.addEventListener("DOMContentLoaded", callback); - } - - return promise; -} - -function getGBrowser() { - return getMostRecentBrowserWindow().gBrowser; -} - -function wait(n) { - let { resolve, promise } = defer(); - setTimeout(resolve, n); - return promise; -} diff --git a/devtools/client/shared/moz.build b/devtools/client/shared/moz.build index 21fcb29e05..814ab75cc4 100644 --- a/devtools/client/shared/moz.build +++ b/devtools/client/shared/moz.build @@ -26,7 +26,6 @@ DevToolsModules( 'devices.js', 'devtools-file-watcher.js', 'DOMHelpers.jsm', - 'doorhanger.js', 'file-watcher-worker.js', 'file-watcher.js', 'getjson.js', diff --git a/devtools/client/webconsole/hudservice.js b/devtools/client/webconsole/hudservice.js index 9aa9e86aaf..284124137f 100644 --- a/devtools/client/webconsole/hudservice.js +++ b/devtools/client/webconsole/hudservice.js @@ -19,7 +19,6 @@ loader.lazyRequireGetter(this, "WebConsoleFrame", "devtools/client/webconsole/we loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true); loader.lazyRequireGetter(this, "DebuggerServer", "devtools/server/main", true); loader.lazyRequireGetter(this, "DebuggerClient", "devtools/shared/client/main", true); -loader.lazyRequireGetter(this, "showDoorhanger", "devtools/client/shared/doorhanger", true); loader.lazyRequireGetter(this, "viewSource", "devtools/client/shared/view-source"); const STRINGS_URI = "devtools/client/locales/webconsole.properties"; From 58525f05f2548ece5c066f56b178df5a23786b97 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Thu, 22 Feb 2024 09:25:02 +0800 Subject: [PATCH 3/6] No issue - Replacing location might throw NS_ERROR_UNEXPECTED while the developer tools window is being destroyed --- devtools/client/framework/toolbox.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 9b9fa66330..fae3602622 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -2182,14 +2182,16 @@ Toolbox.prototype = { .then(() => { this._removeHostListeners(); - // `location` may already be null if the toolbox document is already - // in process of destruction. Otherwise if it is still around, ensure - // releasing toolbox document and triggering cleanup thanks to unload - // event. We do that precisely here, before nullifying the target as - // various cleanup code depends on the target attribute to be still - // defined. - if (win.location) { + // `location` may already be invalid if the toolbox document is + // already in process of destruction. Otherwise if it is still + // around, ensure releasing toolbox document and triggering cleanup + // thanks to unload event. We do that precisely here, before + // nullifying the target as various cleanup code depends on the + // target attribute to be still defined. + try { win.location.replace("about:blank"); + } catch (e) { + // Do nothing. } // Targets need to be notified that the toolbox is being torn down. From d1b0f59de40ff8ba78dd2b4b236030a52c926b7a Mon Sep 17 00:00:00 2001 From: Jens Stutte Date: Thu, 22 Feb 2024 19:53:24 +0100 Subject: [PATCH 4/6] [DOM] Check if WorkerRunnable::Run runs on top of WorkerThreadPrimaryRunnable::Run in a worker thread. --- dom/workers/WorkerPrivate.cpp | 4 +++- dom/workers/WorkerRunnable.cpp | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 85bcc2ffef..fb2e4d7b56 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -5161,7 +5161,9 @@ WorkerPrivate::ScheduleDeletion(WorkerRanOrNot aRanOrNot) if (WorkerRan == aRanOrNot) { nsIThread* currentThread = NS_GetCurrentThread(); MOZ_ASSERT(currentThread); - MOZ_ASSERT(!NS_HasPendingEvents(currentThread)); + // On the worker thread WorkerRunnable will refuse to run if not nested + // on top of a WorkerThreadPrimaryRunnable. + Unused << NS_WARN_IF(NS_HasPendingEvents(currentThread)); } #endif diff --git a/dom/workers/WorkerRunnable.cpp b/dom/workers/WorkerRunnable.cpp index be0171b768..eb56650b51 100644 --- a/dom/workers/WorkerRunnable.cpp +++ b/dom/workers/WorkerRunnable.cpp @@ -11,6 +11,7 @@ #include "nsIRunnable.h" #include "nsThreadUtils.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/DebugOnly.h" #include "mozilla/ErrorResult.h" #include "mozilla/Unused.h" @@ -231,6 +232,19 @@ WorkerRunnable::Run() bool targetIsWorkerThread = mBehavior == WorkerThreadModifyBusyCount || mBehavior == WorkerThreadUnchangedBusyCount; + if (targetIsWorkerThread) { + // On a worker thread, a WorkerRunnable should only run when there is an + // underlying WorkerThreadPrimaryRunnable active, which means we should + // find a CycleCollectedJSContext. + if (!CycleCollectedJSContext::Get()) { + MOZ_DIAGNOSTIC_ASSERT(false, + "A WorkerRunnable was executed after " + "WorkerThreadPrimaryRunnable ended."); + + return NS_OK; + } + } + #ifdef DEBUG MOZ_ASSERT_IF(mCallingCancelWithinRun, targetIsWorkerThread); if (targetIsWorkerThread) { From 2eea07fc65425912e8d07a12b0f511afcd9bce95 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Thu, 22 Feb 2024 20:36:30 +0100 Subject: [PATCH 5/6] [layout] Process pairs in coord list in PolyArea. Range -1 assures the array is even with a step size of 2, avoiding lone value processing. --- layout/generic/nsImageMap.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/layout/generic/nsImageMap.cpp b/layout/generic/nsImageMap.cpp index 19c8797567..bc200a345d 100644 --- a/layout/generic/nsImageMap.cpp +++ b/layout/generic/nsImageMap.cpp @@ -529,7 +529,7 @@ void PolyArea::Draw(nsIFrame* aFrame, DrawTarget& aDrawTarget, Point p1(pc->CSSPixelsToDevPixels(mCoords[0]), pc->CSSPixelsToDevPixels(mCoords[1])); Point p2, p1snapped, p2snapped; - for (int32_t i = 2; i < mNumCoords; i += 2) { + for (int32_t i = 2; i < mNumCoords - 1; i += 2) { p2.x = pc->CSSPixelsToDevPixels(mCoords[i]); p2.y = pc->CSSPixelsToDevPixels(mCoords[i+1]); p1snapped = p1; @@ -556,7 +556,7 @@ void PolyArea::GetRect(nsIFrame* aFrame, nsRect& aRect) nscoord x1, x2, y1, y2, xtmp, ytmp; x1 = x2 = nsPresContext::CSSPixelsToAppUnits(mCoords[0]); y1 = y2 = nsPresContext::CSSPixelsToAppUnits(mCoords[1]); - for (int32_t i = 2; i < mNumCoords; i += 2) { + for (int32_t i = 2; i < mNumCoords - 1; i += 2) { xtmp = nsPresContext::CSSPixelsToAppUnits(mCoords[i]); ytmp = nsPresContext::CSSPixelsToAppUnits(mCoords[i+1]); x1 = x1 < xtmp ? x1 : xtmp; From 9d320dac804653df54ad62da2c87905677f7c258 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 22 Feb 2024 22:08:23 +0100 Subject: [PATCH 6/6] [network] Block set-cookie from multipart/x-mixed-replace --- netwerk/streamconv/converters/nsMultiMixedConv.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/netwerk/streamconv/converters/nsMultiMixedConv.cpp b/netwerk/streamconv/converters/nsMultiMixedConv.cpp index 4ebd61d9c2..44a0dfd705 100644 --- a/netwerk/streamconv/converters/nsMultiMixedConv.cpp +++ b/netwerk/streamconv/converters/nsMultiMixedConv.cpp @@ -1018,11 +1018,14 @@ nsMultiMixedConv::ParseHeaders(nsIChannel *aChannel, char *&aPtr, } else if (headerStr.LowerCaseEqualsLiteral("content-disposition")) { mContentDisposition = headerVal; } else if (headerStr.LowerCaseEqualsLiteral("set-cookie")) { + // Do not allow cookies to be set from multipart/mixed content parts + // as a mitigation to CVE-2024-1551 + /* nsCOMPtr httpInternal = do_QueryInterface(aChannel); if (httpInternal) { httpInternal->SetCookie(headerVal.get()); - } + }*/ } else if (headerStr.LowerCaseEqualsLiteral("content-range") || headerStr.LowerCaseEqualsLiteral("range") ) { // something like: Content-range: bytes 7000-7999/8000