diff --git a/www/qt6-webengine/Makefile b/www/qt6-webengine/Makefile index 7153864dad46..9c9c5b0c5a25 100644 --- a/www/qt6-webengine/Makefile +++ b/www/qt6-webengine/Makefile @@ -1,167 +1,167 @@ # QtWebEngine itself is a very thin layer of Qt code on top of a large part of # Chromium (everything up to the content/ layer). As such, most of the work in # this port revolves around taming Chromium and getting it to build on FreeBSD. # While it does build at the moment, there are several items that should be # investigated or improved: # - We are using several stub files, especially in Chromium's base/ and net/ # layers. We should look at implementing the missing bits instead. # - We are currently not using any sandboxing mechanism. # - The process of porting QtWebEngine needs to be documented so we can move to # newer releases more easily. # PORTNAME?= webengine DISTVERSION= ${QT6_VERSION} -PORTREVISION?= 0 # Master port for print/qt6-pdf. Please keep this line. +PORTREVISION?= 1 # Master port for print/qt6-pdf. Please keep this line. CATEGORIES?= www PKGNAMEPREFIX= qt6- MAINTAINER= kde@FreeBSD.org COMMENT?= Qt 6 library to render web content BUILD_DEPENDS= ${PYTHON_PKGNAMEPREFIX}html5lib>0:www/py-html5lib@${PY_FLAVOR} \ ${LOCALBASE}/include/vulkan/vulkan.h:graphics/vulkan-headers LIB_DEPENDS= libfreetype.so:print/freetype2 \ libnss3.so:security/nss \ libxkbcommon.so:x11/libxkbcommon USES= bison cmake compiler:c++17-lang gl gperf \ localbase:ldflags ninja:build nodejs:build,lts \ pkgconfig python:build qt-dist:6,webengine USE_GL= opengl USE_QT= base declarative tools USE_XORG= x11 xcb xcomposite xcursor xdamage xext xfixes xi xkbfile \ xrandr xrender xscrnsaver xshmfence xtst CMAKE_ON= QT_FEATURE_webengine_system_ffmpeg \ QT_FEATURE_webengine_proprietary_codecs .if defined(BUILD_QTPDF) LIB_DEPENDS+= libcups.so:print/cups \ libopenjp2.so:graphics/openjpeg CMAKE_ON+= QT_FEATURE_qtpdf_build CMAKE_OFF+= QT_FEATURE_qtwebengine_build SYS_LIBS= freetype .else BUILD_DEPENDS+= ${LOCALBASE}/include/linux/videodev2.h:multimedia/v4l_compat LIB_DEPENDS+= libavcodec.so:multimedia/ffmpeg \ libdbus-1.so:devel/dbus \ libdrm.so:graphics/libdrm \ libevent.so:devel/libevent \ libexpat.so:textproc/expat2 \ libfontconfig.so:x11-fonts/fontconfig \ libharfbuzz.so:print/harfbuzz \ libnspr4.so:devel/nspr \ libopenh264.so:multimedia/openh264 \ libopus.so:audio/opus \ libpci.so:devel/libpci \ libpng.so:graphics/png \ libre2.so:devel/re2 \ libsnappy.so:archivers/snappy \ libvpx.so:multimedia/libvpx \ libwebp.so:graphics/webp USES+= gnome jpeg minizip xorg USE_GL+= gbm USE_QT+= positioning quick3d:build webchannel USE_GNOME+= glib20 libxml2 libxslt USE_XORG+= x11 xcb xcomposite xcursor xdamage xext xfixes xi xkbfile \ xrandr xrender xscrnsaver xshmfence xtst CMAKE_ON+= QT_FEATURE_qtwebengine_build CMAKE_OFF+= QT_FEATURE_qtpdf_build SYS_LIBS= freetype harfbuzz-ng libdrm libevent libpng libxml libxslt openh264 opus .endif # The build system reads the environment variable $NINJA_PATH to decide whether # to boostrap ninja or not (and also to invoke it afterwards). CC and CXX are # read by some Chromium code to determine which compiler to invoke when running # some configuration tests. CONFIGURE_ENV+= NINJAFLAGS="-j${MAKE_JOBS_NUMBER}" \ NINJA_PATH="${LOCALBASE}/bin/ninja" \ PATH=${CONFIGURE_WRKSRC}/bin:${LOCALBASE}/bin:${PATH} MAKE_ENV+= CC="${CC}" CXX="${CXX}" \ C_INCLUDE_PATH=${LOCALBASE}/include \ CPLUS_INCLUDE_PATH=${LOCALBASE}/include \ ${CONFIGURE_ENV} # Avoid running multiple make(1) jobs, but only those. Otherwise the build # fails intermittently due race conditions if multiple ninja instances are # running at the same time (mostly for the targets "WebEngineCore" and # "convert_dict"). # # MAKE_JOBS_UNSAFE is too invasive because it also affects the number of jobs # for ninja(1) and would slow everything down which we don't want. We pass the # real number of make jobs via MAKE_JOBS_NUMBER to ninja(1) to CONFIGURE_ENV. DO_MAKE_BUILD= ${SETENV} ${MAKE_ENV} ${MAKE_CMD} ${MAKE_FLAGS} ${MAKEFILE} -j1 ${MAKE_ARGS:N${DESTDIRNAME}=*} BINARY_ALIAS= python3=${PYTHON_CMD} .if !defined(BUILD_QTPDF) OPTIONS_DEFINE= CUPS OPTIONS_DEFAULT= ALSA CUPS OPTIONS_SINGLE= AUDIO OPTIONS_SINGLE_AUDIO= ALSA PULSEAUDIO SNDIO AUDIO_DESC= Audio backend # Need the alsa plugins to get sound at runtime, otherwise messages # that the pcm_oss plugin can't be opened. ALSA_LIB_DEPENDS= libasound.so:audio/alsa-lib ALSA_RUN_DEPENDS= alsa-plugins>=0:audio/alsa-plugins ALSA_CMAKE_ON= -DQT_FEATURE_webengine_system_alsa:BOOL=ON ALSA_CMAKE_OFF= -DQT_FEATURE_webengine_system_alsa:BOOL=OFF CUPS_LIB_DEPENDS= libcups.so:print/cups \ liblcms2.so:graphics/lcms2 \ libopenjp2.so:graphics/openjpeg \ libtiff.so:graphics/tiff CUPS_CMAKE_ON= -DQT_FEATURE_webengine_printing_and_pdf:BOOL=ON CUPS_CMAKE_OFF= -DQT_FEATURE_webengine_printing_and_pdf:BOOL=OFF PULSEAUDIO_LIB_DEPENDS= libpulse.so:audio/pulseaudio PULSEAUDIO_CMAKE_ON= -DQT_FEATURE_webengine_system_pulseaudio:BOOL=ON PULSEAUDIO_CMAKE_OFF= -DQT_FEATURE_webengine_system_pulseaudio:BOOL=OFF SNDIO_LIB_DEPENDS= libsndio.so:audio/sndio SNDIO_CMAKE_ON= -DQT_FEATURE_webengine_system_sndio:BOOL=ON SNDIO_CMAKE_OFF= -DQT_FEATURE_webengine_system_sndio:BOOL=OFF .endif .include .if ${ARCH} == i386 || ${ARCH} == armv7 EXTRA_PATCHES+= ${PATCHDIR}/extra-patch-32bit-compress_files_js .endif post-extract: @${MKDIR} ${WRKSRC}/src/3rdparty/chromium/media/audio/sndio \ ${WRKSRC}/src/3rdparty/chromium/sandbox/policy/freebsd \ ${WRKSRC}/src/3rdparty/chromium/sandbox/policy/openbsd post-patch: @${REINPLACE_CMD} -e 's|%%CPPFLAGS%%|${CPPFLAGS}|;s|%%CXXFLAGS%%|${CXXFLAGS}|;s|%%LDFLAGS%%|${LDFLAGS}|' \ ${WRKSRC}/src/host/BUILD.toolchain.gn.in @${REINPLACE_CMD} -e 's|%%LOCALBASE%%|${LOCALBASE}|' \ ${WRKSRC}/src/3rdparty/chromium/third_party/pdfium/core/fxge/linux/fx_linux_impl.cpp \ ${WRKSRC}/src/3rdparty/chromium/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc pre-configure: # We used to remove bundled libraries to be sure that webengine uses # system libraries and not shipped ones. # # Leads to missing header errors: icu, libvpx, libwebp, re2, snappy, zlib # No-Op: flac # # Don't attempt to unbundle libraries that the Pdf module doesn't use. It # causes configuration errors. # # cd ${WRKSRC} && ${PYTHON_CMD} \ # ./build/linux/unbundle/remove_bundled_libraries.py [list of preserved] cd ${WRKSRC}/src/3rdparty/chromium && ${SETENV} ${CONFIGURE_ENV} ${PYTHON_CMD} \ ./build/linux/unbundle/replace_gn_files.py --system-libraries \ ${SYS_LIBS} || ${FALSE} .include diff --git a/www/qt6-webengine/files/patch-security-rollup b/www/qt6-webengine/files/patch-security-rollup index a50454f4e40c..8b32c0fe79cf 100644 --- a/www/qt6-webengine/files/patch-security-rollup +++ b/www/qt6-webengine/files/patch-security-rollup @@ -1,103 +1,655 @@ Add security patches to this file. Addresses the following security issues: - CVE-2023-5997 - CVE-2023-6112 +- CVE-2023-6345 +- CVE-2023-6346 +- CVE-2023-6347 +- CVE-2023-6510 +- Security bug 1485266 From 669506a53474e3d7637666d3c53f6101fb94d96f Mon Sep 17 00:00:00 2001 From: Nidhi Jaju Date: Thu, 2 Nov 2023 08:16:57 +0000 Subject: [PATCH] [Backport] CVE-2023-5997: Use after free in Garbage Collection Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4996929: Make URLSearchParams persistent to avoid UaF The URLSearchParams::Create() function returns an on-heap object, but it can be garbage collected, so making it a persistent variable in DidFetchDataLoadedString() mitigates the issue. Bug: 1497997 Change-Id: I229efec33451792a10a185cb2f9aa37dd0579823 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4996929 Reviewed-by: Adam Rice Commit-Queue: Nidhi Jaju Cr-Commit-Position: refs/heads/main@{#1218682} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/518606 Reviewed-by: Michal Klocek --- chromium/third_party/blink/renderer/core/fetch/body.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/chromium/third_party/blink/renderer/core/fetch/body.cc b/chromium/third_party/blink/renderer/core/fetch/body.cc index f24125ee271..6f6abd5b961 100644 --- src/3rdparty/chromium/third_party/blink/renderer/core/fetch/body.cc.orig +++ src/3rdparty/chromium/third_party/blink/renderer/core/fetch/body.cc @@ -119,8 +119,13 @@ class BodyFormDataConsumer final : public BodyConsumerBase { void DidFetchDataLoadedString(const String& string) override { auto* formData = MakeGarbageCollected(); - for (const auto& pair : URLSearchParams::Create(string)->Params()) + // URLSearchParams::Create() returns an on-heap object, but it can be + // garbage collected, so making it a persistent variable on the stack + // mitigates use-after-free scenarios. See crbug.com/1497997. + Persistent search_params = URLSearchParams::Create(string); + for (const auto& pair : search_params->Params()) { formData->append(pair.first, pair.second); + } DidFetchDataLoadedFormData(formData); } }; From 6c805bf7507997616d826f20c7c901739ed3b431 Mon Sep 17 00:00:00 2001 From: Yoshisato Yanagisawa Date: Tue, 7 Nov 2023 02:56:57 +0000 Subject: [PATCH] [Backport] CVE-2023-6112: Use after free in Navigation Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5004329: Use WeakPointer for the loader fallback callback. `MaybeStartLoader` binds an unretained pointer to `FallbackToNonInterceptedRequest`, which is passed through a series of objects until it reaches `ServiceWorkerMainResourceLoader`. When "network" or "cache" is selected as a ServiceWorker static routing API's source and caused the network fallback, the unretained pointer can be released and may cause use-after-free. This CL changes the unretained pointer to a weak pointer to avoid that. For the consistency of the function call, both of the callbacks starts to use weak pointers. Bug: 1499298 Change-Id: I7e7c93fa389ab35584703f30bfc722eadeca81dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5004329 Reviewed-by: Shunya Shishido Reviewed-by: Minoru Chikamune Reviewed-by: Takashi Toyoshima Commit-Queue: Yoshisato Yanagisawa Cr-Commit-Position: refs/heads/main@{#1220697} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/518607 Reviewed-by: Michal Klocek --- chromium/content/browser/loader/navigation_url_loader_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chromium/content/browser/loader/navigation_url_loader_impl.cc b/chromium/content/browser/loader/navigation_url_loader_impl.cc index 0e8f73e7d18..0bd83dadec2 100644 --- src/3rdparty/chromium/content/browser/loader/navigation_url_loader_impl.cc.orig +++ src/3rdparty/chromium/content/browser/loader/navigation_url_loader_impl.cc @@ -638,10 +638,10 @@ void NavigationURLLoaderImpl::MaybeStartLoader( next_interceptor->MaybeCreateLoader( *resource_request_, browser_context_, base::BindOnce(&NavigationURLLoaderImpl::MaybeStartLoader, - base::Unretained(this), next_interceptor), + weak_factory_.GetWeakPtr(), next_interceptor), base::BindOnce( &NavigationURLLoaderImpl::FallbackToNonInterceptedRequest, - base::Unretained(this))); + weak_factory_.GetWeakPtr())); return; } +From d997551c21008fb8d9f5fe9ffe5506af6273ea49 Mon Sep 17 00:00:00 2001 +From: John Stiles +Date: Fri, 24 Nov 2023 09:40:11 -0500 +Subject: [PATCH] [Backport] CVE-2023-6345: Integer overflow in Skia (1/2) + +Cherry-pick of patch originally reviewed on +https://skia-review.googlesource.com/c/skia/+/782936: +Avoid combining extremely large meshes. + +Bug: chromium:1505053 +Change-Id: I42f2ff872bbf054686ec7af0cc85ff63055fcfbf +Reviewed-on: https://skia-review.googlesource.com/c/skia/+/782936 +Commit-Queue: Michael Ludwig +Reviewed-by: Michael Ludwig +Auto-Submit: John Stiles +(cherry picked from commit 6169a1fabae1743709bc9641ad43fcbb6a4f62e1) +Reviewed-on: https://skia-review.googlesource.com/c/skia/+/783296 +Reviewed-by: John Stiles +Commit-Queue: Brian Osman +Auto-Submit: Brian Osman +Commit-Queue: John Stiles +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/522251 +Reviewed-by: Michal Klocek +--- + chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp b/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp +index 9b38c0bdb61..4dc885a7431 100644 +--- src/3rdparty/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp.orig ++++ src/3rdparty/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp +@@ -998,10 +998,13 @@ GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const Gr + return CombineResult::kCannotCombine; + } + ++ if (fVertexCount > INT32_MAX - that->fVertexCount) { ++ return CombineResult::kCannotCombine; ++ } + if (SkToBool(fIndexCount) != SkToBool(that->fIndexCount)) { + return CombineResult::kCannotCombine; + } +- if (SkToBool(fIndexCount) && fVertexCount + that->fVertexCount > SkToInt(UINT16_MAX)) { ++ if (SkToBool(fIndexCount) && fVertexCount > UINT16_MAX - that->fVertexCount) { + return CombineResult::kCannotCombine; + } + +From 297e07a3f4008da601f6190e65c5c0368a7a7997 Mon Sep 17 00:00:00 2001 +From: John Stiles +Date: Sat, 25 Nov 2023 22:41:31 -0500 +Subject: [PATCH] [Backport] CVE-2023-6345: Integer overflow in Skia (2/2) + +Cherry-pick of patch originally reviewed on +https://skia-review.googlesource.com/c/skia/+/783036: +Use SkToInt to avoid warning in Flutter roll. + +The Flutter roll was failing due to -Wsign-compare. + +Bug: chromium:1505053 +Change-Id: Id12876f6f97682466f19b56cfa562366380f27cb +Reviewed-on: https://skia-review.googlesource.com/c/skia/+/783036 +Auto-Submit: John Stiles +Commit-Queue: Brian Osman +Reviewed-by: Brian Osman +(cherry picked from commit 0eea0b277d7d35e4c2612646d7dfe507341e337e) +Reviewed-on: https://skia-review.googlesource.com/c/skia/+/782579 +Commit-Queue: John Stiles +Reviewed-by: John Stiles +Auto-Submit: Brian Osman +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/522252 +Reviewed-by: Michal Klocek +--- + chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp b/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp +index 4dc885a7431..d594abec6dd 100644 +--- src/3rdparty/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp.orig ++++ src/3rdparty/chromium/third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp +@@ -1004,7 +1004,7 @@ GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const Gr + if (SkToBool(fIndexCount) != SkToBool(that->fIndexCount)) { + return CombineResult::kCannotCombine; + } +- if (SkToBool(fIndexCount) && fVertexCount > UINT16_MAX - that->fVertexCount) { ++ if (SkToBool(fIndexCount) && fVertexCount > SkToInt(UINT16_MAX) - that->fVertexCount) { + return CombineResult::kCannotCombine; + } + +From 41b5dbaa659003d91ebf1b1018201d3cb76d4486 Mon Sep 17 00:00:00 2001 +From: Ken Rockot +Date: Thu, 16 Nov 2023 23:23:22 +0000 +Subject: [PATCH] [Backport] CVE-2023-6347: Use after free in Mojo + +Cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5038080: +Reland: Fix IPC Channel pipe teardown + +This is a reland with the new test temporarily disabled on Android +until it can run without disrupting other tests. + +(cherry picked from commit cd4c1f165c16c6d8161b5372ef7f61c715e01a42) + +Fixed: 1494461 +Change-Id: If1d83c2dce62020f78dd50abc460973759002a1a +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5015115 +Commit-Queue: Ken Rockot +Reviewed-by: Robert Sesek +Cr-Original-Commit-Position: refs/heads/main@{#1221953} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5038080 +Auto-Submit: Ken Rockot +Commit-Queue: Daniel Cheng +Reviewed-by: Daniel Cheng +Cr-Commit-Position: refs/branch-heads/6045@{#1383} +Cr-Branched-From: 905e8bdd32d891451d94d1ec71682e989da2b0a1-refs/heads/main@{#1204232} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/522253 +Reviewed-by: Michal Klocek +--- + chromium/ipc/ipc_mojo_bootstrap.cc | 43 ++++++++++++++++++++++-------- + 1 file changed, 32 insertions(+), 11 deletions(-) + +diff --git a/chromium/ipc/ipc_mojo_bootstrap.cc b/chromium/ipc/ipc_mojo_bootstrap.cc +index b9b5ec389aa..5391400cdb0 100644 +--- src/3rdparty/chromium/ipc/ipc_mojo_bootstrap.cc.orig ++++ src/3rdparty/chromium/ipc/ipc_mojo_bootstrap.cc +@@ -793,13 +793,12 @@ class ChannelAssociatedGroupController + // handle. + DCHECK(!endpoint->client()); + DCHECK(endpoint->peer_closed()); +- MarkClosedAndMaybeRemove(endpoint); ++ MarkClosed(endpoint); + } else { +- MarkPeerClosedAndMaybeRemove(endpoint); ++ MarkPeerClosed(endpoint); + } + } +- +- DCHECK(endpoints_.empty()); ++ endpoints_.clear(); + + GetMemoryDumpProvider().RemoveController(this); + } +@@ -844,15 +843,19 @@ class ChannelAssociatedGroupController + base::AutoLock locker(lock_); + encountered_error_ = true; + ++ std::vector endpoints_to_remove; + std::vector> endpoints_to_notify; + for (auto iter = endpoints_.begin(); iter != endpoints_.end();) { + Endpoint* endpoint = iter->second.get(); + ++iter; + +- if (endpoint->client()) ++ if (endpoint->client()) { + endpoints_to_notify.push_back(endpoint); ++ } + +- MarkPeerClosedAndMaybeRemove(endpoint); ++ if (MarkPeerClosed(endpoint)) { ++ endpoints_to_remove.push_back(endpoint->id()); ++ } + } + + for (auto& endpoint : endpoints_to_notify) { +@@ -861,6 +864,10 @@ class ChannelAssociatedGroupController + if (endpoint->client()) + NotifyEndpointOfError(endpoint.get(), false /* force_async */); + } ++ ++ for (uint32_t id : endpoints_to_remove) { ++ endpoints_.erase(id); ++ } + } + + void NotifyEndpointOfError(Endpoint* endpoint, bool force_async) { +@@ -899,19 +906,33 @@ class ChannelAssociatedGroupController + NotifyEndpointOfError(endpoint, false /* force_async */); + } + +- void MarkClosedAndMaybeRemove(Endpoint* endpoint) { ++ // Marks `endpoint` as closed and returns true if and only if its peer was ++ // also already closed. ++ bool MarkClosed(Endpoint* endpoint) { + lock_.AssertAcquired(); + endpoint->set_closed(); +- if (endpoint->closed() && endpoint->peer_closed()) +- endpoints_.erase(endpoint->id()); ++ return endpoint->peer_closed(); + } + +- void MarkPeerClosedAndMaybeRemove(Endpoint* endpoint) { ++ // Marks `endpoint` as having a closed peer and returns true if and only if ++ // `endpoint` itself was also already closed. ++ bool MarkPeerClosed(Endpoint* endpoint) { + lock_.AssertAcquired(); + endpoint->set_peer_closed(); + endpoint->SignalSyncMessageEvent(); +- if (endpoint->closed() && endpoint->peer_closed()) ++ return endpoint->closed(); ++ } ++ ++ void MarkClosedAndMaybeRemove(Endpoint* endpoint) { ++ if (MarkClosed(endpoint)) { + endpoints_.erase(endpoint->id()); ++ } ++ } ++ ++ void MarkPeerClosedAndMaybeRemove(Endpoint* endpoint) { ++ if (MarkPeerClosed(endpoint)) { ++ endpoints_.erase(endpoint->id()); ++ } + } + + Endpoint* FindOrInsertEndpoint(mojo::InterfaceId id, bool* inserted) { +From 148f39658c9977dcdfe8a51e212ce936f246dcfc Mon Sep 17 00:00:00 2001 +From: Alvin Ji +Date: Fri, 17 Nov 2023 00:56:14 +0000 +Subject: [PATCH] [Backport] CVE-2023-6346: Use after free in WebAudio + +Manual cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5037917: +Check context status before creating new platform destination + +RealtimeAudioDestinationHandler::SetSinkDescriptor creates new +destination platofrm without validating context status. This can +reactivate the audio rendering thread when AudioContext is already in +closed state. + +(cherry picked from commit 0f9bb9a1083865d4e51059e588f27f729ab32753) + +Bug: 1500856 +Change-Id: If1fd531324b56fcdc38d315fd84d4cec577a14bc +Test: Locally confirmed with ASAN +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5021160 +Reviewed-by: Alvin Ji +Commit-Queue: Alvin Ji +Reviewed-by: Hongchan Choi +Cr-Original-Commit-Position: refs/heads/main@{#1223168} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5037917 +Bot-Commit: Rubber Stamper +Commit-Queue: Hongchan Choi +Cr-Commit-Position: refs/branch-heads/5993@{#1619} +Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/522254 +Reviewed-by: Michal Klocek +--- + .../webaudio/realtime_audio_destination_handler.cc | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc b/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc +index 8cc1d9dadcb..0cde579951a 100644 +--- src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc.orig ++++ src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/realtime_audio_destination_handler.cc +@@ -398,6 +398,17 @@ void RealtimeAudioDestinationHandler::SetSinkDescriptor( + MaxChannelCount(), GetCallbackBufferSize())); + DCHECK(IsMainThread()); + ++ // After the context is closed, `SetSinkDescriptor` request will be ignored ++ // because it will trigger the recreation of the platform destination. This in ++ // turn can activate the audio rendering thread. ++ AudioContext* context = static_cast(Context()); ++ CHECK(context); ++ if (context->ContextState() == AudioContext::kClosed) { ++ std::move(callback).Run( ++ media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_ERROR_INTERNAL); ++ return; ++ } ++ + // Create a pending AudioDestination to replace the current one. + scoped_refptr pending_platform_destination = + AudioDestination::Create( +From db834bc30340727483633a92bbf27eb60839a56f Mon Sep 17 00:00:00 2001 +From: Jordan Bayles +Date: Fri, 6 Oct 2023 23:50:59 +0000 +Subject: [PATCH] [Backport] CVE-2023-6510: Use after free in Media Capture + +Manual cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/4908770: +Fix UaF in WebContentsFrameTracker + +This patch fixes a use-after-free by moving to a base::WeakPtr +instead of a raw_ptr. Looking at the callstack in the referenced bug, what is clearly happening is that the frame tracker is deleted AFTER the capture device. I believe that this is due to the MouseCursorOverlayController being deleted through the DeleteOnUIThread destructor, which, if you are already on the UI thread, is synchronous: + +https://source.chromium.org/chromium/chromium/src/+/main:content/public/browser/browser_thread.h;l=141?q=BrowserThread::DeleteOnThread&ss=chromium%2Fchromium%2Fsrc + +In comparison, the WebContentsFrameTracker is implemented using base::SequenceBound, which ends up calling an internal destruct method that ALWAYS posts back a task: + +https://source.chromium.org/chromium/chromium/src/+/main:base/threading/sequence_bound_internal.h;drc=f5bdc89c7395ed24f1b8d196a3bdd6232d5bf771;l=122 + +So, this bug is ultimately caused by the simple fact that base::SequenceBound does NOT have an optimization to not post a deletion task if we are already running on that sequence. There may be a good followup task here to change either DeleteOnThread or base::SequenceBound to have the same behavior, however I think this change a good first step. + +Bug: 1480152 +Change-Id: Iee2d41e66b10403d6c78547bcbe84d2454236d5b +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908770 +Reviewed-by: Mark Foltz +Commit-Queue: Jordan Bayles +Cr-Commit-Position: refs/heads/main@{#1206698} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/523710 +Reviewed-by: Michal Klocek +--- + .../media/capture/web_contents_frame_tracker.cc | 17 +++++++++++------ + .../media/capture/web_contents_frame_tracker.h | 11 +++++------ + 2 files changed, 16 insertions(+), 12 deletions(-) + +diff --git a/chromium/content/browser/media/capture/web_contents_frame_tracker.cc b/chromium/content/browser/media/capture/web_contents_frame_tracker.cc +index 353f47f24af..9e3e3e82809 100644 +--- src/3rdparty/chromium/content/browser/media/capture/web_contents_frame_tracker.cc.orig ++++ src/3rdparty/chromium/content/browser/media/capture/web_contents_frame_tracker.cc +@@ -126,17 +126,20 @@ WebContentsFrameTracker::WebContentsFrameTracker( + base::WeakPtr device, + MouseCursorOverlayController* cursor_controller) + : device_(std::move(device)), +- device_task_runner_(std::move(device_task_runner)) { ++ device_task_runner_(std::move(device_task_runner)) ++#if !BUILDFLAG(IS_ANDROID) ++ , ++ cursor_controller_(cursor_controller->GetWeakPtr()) ++#endif ++{ + // Verify on construction that this object is created on the UI thread. After + // this, depend on the sequence checker to ensure consistent execution. + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); +- +- DCHECK(device_task_runner_); ++ CHECK(device_task_runner_); + + #if !BUILDFLAG(IS_ANDROID) +- cursor_controller_ = cursor_controller; +- DCHECK(cursor_controller_); ++ CHECK(cursor_controller_); + #endif + } + +@@ -516,7 +519,9 @@ void WebContentsFrameTracker::SetTargetView(gfx::NativeView view) { + return; + target_native_view_ = view; + #if !BUILDFLAG(IS_ANDROID) +- cursor_controller_->SetTargetView(view); ++ if (cursor_controller_) { ++ cursor_controller_->SetTargetView(view); ++ } + #endif + } + +diff --git a/chromium/content/browser/media/capture/web_contents_frame_tracker.h b/chromium/content/browser/media/capture/web_contents_frame_tracker.h +index f15b09619de..c6485cc6fdf 100644 +--- src/3rdparty/chromium/content/browser/media/capture/web_contents_frame_tracker.h.orig ++++ src/3rdparty/chromium/content/browser/media/capture/web_contents_frame_tracker.h +@@ -171,13 +171,12 @@ class CONTENT_EXPORT WebContentsFrameTracker final + // The task runner to be used for device callbacks. + const scoped_refptr device_task_runner_; + +- // Owned by FrameSinkVideoCaptureDevice. This will be valid for the life of +- // WebContentsFrameTracker because the WebContentsFrameTracker deleter task +- // will be posted to the UI thread before the MouseCursorOverlayController +- // deleter task. ++ // Owned by FrameSinkVideoCaptureDevice. This may only be accessed on the ++ // UI thread. This is not guaranteed to be valid and must be checked before ++ // use. ++ // https://crbug.com/1480152 + #if !BUILDFLAG(IS_ANDROID) +- raw_ptr cursor_controller_ = +- nullptr; ++ const base::WeakPtr cursor_controller_; + #endif + + // We may not have a frame sink ID target at all times. +From d8d7dc06d0423ad9fdcbe23e741c24b560ff97b8 Mon Sep 17 00:00:00 2001 +From: Evan Stade +Date: Wed, 4 Oct 2023 00:08:36 +0000 +Subject: [PATCH] [Backport] Security bug 1485266 + +Manual cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/4902775: +Drag and drop: prevent cross-origin same-tab drags that span navigations + +In IsValidDragTarget, the old RenderViewHostID comparison was not +necessary to distinguish between same- and different-tab drags because, +contrary to the previous comment, that case is covered by the +`drag_start_` check. This check was only serving to permit some drags +which were same-tab, but not same-RVH, which should be disallowed. + +A complete rundown of the business logic and the reason for the +business logic is here: +https://bugs.chromium.org/p/chromium/issues/detail?id=1266953#c22 + +A regression test is added which is confirmed to fail without this fix, +but only on Chrome OS because that's the only Aura platform where the +DND interactive UI tests are not already disabled (Windows and Linux +were disabled). + +Bug: 1485266 +Change-Id: Ifdd6eec14df42372b0afc8ccba779a948cbaaaa7 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4902775 +Commit-Queue: Evan Stade +Reviewed-by: Daniel Cheng +Reviewed-by: Charlie Reis +Cr-Commit-Position: refs/heads/main@{#1204930} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/523711 +Reviewed-by: Michal Klocek +--- + .../web_contents/web_contents_view_aura.cc | 44 ++++++------------- + .../web_contents/web_contents_view_aura.h | 26 +++-------- + 2 files changed, 20 insertions(+), 50 deletions(-) + +diff --git a/chromium/content/browser/web_contents/web_contents_view_aura.cc b/chromium/content/browser/web_contents/web_contents_view_aura.cc +index 37b75adc1ef..c96e932aacc 100644 +--- src/3rdparty/chromium/content/browser/web_contents/web_contents_view_aura.cc.orig ++++ src/3rdparty/chromium/content/browser/web_contents/web_contents_view_aura.cc +@@ -765,13 +765,10 @@ void WebContentsViewAura::PrepareDropData( + // Do not add FileContents if this is a tainted-cross-origin same-page image + // (https://crbug.com/1264873). + bool access_allowed = +- // Drag started outside blink. + !drag_start_ || +- // Drag began in blink, but image access is allowed. +- drag_start_->image_accessible_from_frame || +- // Drag began in blink, but in a different WebContents. +- GetRenderViewHostID(web_contents_->GetRenderViewHost()) != +- drag_start_->view_id; ++ // Drag began in this top-level WebContents, and image access is allowed ++ // (not cross-origin). ++ drag_start_->image_accessible_from_frame; + data.GetFilenames(&drop_data->filenames); + if (access_allowed && drop_data->filenames.empty()) { + base::FilePath filename; +@@ -887,6 +884,8 @@ bool WebContentsViewAura::IsValidDragTarget( + // drags between cross-origin frames within the same page. Otherwise, a + // malicious attacker could abuse drag interactions to leak information + // across origins without explicit user intent. ++ // `drag_start_` is null when the drag started outside of the browser or from ++ // a different top-level WebContents. + if (!drag_start_) + return true; + +@@ -894,35 +893,19 @@ bool WebContentsViewAura::IsValidDragTarget( + // perform the check unless it already has access to the starting + // document's origin. If the SiteInstanceGroups match, then the process + // allocation policy decided that it is OK for the source and target +- // frames to live in the same renderer process. Furthermore, it means that +- // either the source and target frame are part of the same `blink::Page` or +- // that there is an opener relationship and would cross tab boundaries. Allow +- // this drag to the renderer. Blink will perform an additional check against ++ // frames to live in the same renderer process. Furthermore, having matching ++ // SiteInstanceGroups means that either (1) the source and target frame are ++ // part of the same blink::Page, or (2) that they are in the same Browsing ++ // Context Group and the drag would cross tab boundaries (the latter of which ++ // can't happen here since `drag_start_` is null). Allow this drag to the ++ // renderer. Blink will perform an additional check against + // `blink::DragController::drag_initiator_` to decide whether or not to + // allow the drag operation. This can be done in the renderer, as the + // browser-side checks only have local tree fragment (potentially with + // multiple origins) granularity at best, but a drag operation eventually + // targets one single frame in that local tree fragment. +- bool same_site_instance_group = target_rwh->GetSiteInstanceGroup()->GetId() == +- drag_start_->site_instance_group_id; +- if (same_site_instance_group) +- return true; +- +- // Otherwise, if the SiteInstanceGroups do not match, enforce explicit +- // user intent by ensuring this drag operation is crossing page boundaries. +- // `drag_start_->view_id` is set to the main `RenderFrameHost`'s +- // `RenderViewHost`'s ID when a drag starts, so if the two IDs match here, +- // the drag is within the same page and disallowed. +- // +- // Drags between an embedder and an inner `WebContents` will disallowed by +- // the above view ID check because `WebContentsViewAura` is always created +- // for the outermost view. Inner `WebContents` will have a +- // `WebContentsViewChildFrame` so when dragging between an inner +- // `WebContents` and its embedder the view IDs will be the same. +- bool cross_tab_drag = +- GetRenderViewHostID(web_contents_->GetRenderViewHost()) != +- drag_start_->view_id; +- return cross_tab_drag; ++ return target_rwh->GetSiteInstanceGroup()->GetId() == ++ drag_start_->site_instance_group_id; + } + + //////////////////////////////////////////////////////////////////////////////// +@@ -1180,7 +1163,6 @@ void WebContentsViewAura::StartDragging( + + drag_start_ = + DragStart(source_rwh->GetSiteInstanceGroup()->GetId(), +- GetRenderViewHostID(web_contents_->GetRenderViewHost()), + drop_data.file_contents_image_accessible); + + ui::TouchSelectionController* selection_controller = GetSelectionController(); +diff --git a/chromium/content/browser/web_contents/web_contents_view_aura.h b/chromium/content/browser/web_contents/web_contents_view_aura.h +index dc308525002..48d30860e5e 100644 +--- src/3rdparty/chromium/content/browser/web_contents/web_contents_view_aura.h.orig ++++ src/3rdparty/chromium/content/browser/web_contents/web_contents_view_aura.h +@@ -162,7 +162,7 @@ class CONTENT_EXPORT WebContentsViewAura + + // Returns whether |target_rwh| is a valid RenderWidgetHost to be dragging + // over. This enforces that same-page, cross-site drags are not allowed. See +- // crbug.com/666858. ++ // crbug.com/666858, crbug.com/1266953, crbug.com/1485266. + bool IsValidDragTarget(RenderWidgetHostImpl* target_rwh) const; + + // Called from CreateView() to create |window_|. +@@ -342,7 +342,7 @@ class CONTENT_EXPORT WebContentsViewAura + std::unique_ptr window_observer_; + + // The WebContentsImpl whose contents we display. +- raw_ptr web_contents_; ++ const raw_ptr web_contents_; + + std::unique_ptr delegate_; + +@@ -360,33 +360,21 @@ class CONTENT_EXPORT WebContentsViewAura + // avoid sending the drag exited message after leaving the current view. + GlobalRoutingID current_rvh_for_drag_; + +- // We track the IDs of the source RenderProcessHost and RenderViewHost from +- // which the current drag originated. These are used to ensure that drag +- // events do not fire over a cross-site frame (with respect to the source +- // frame) in the same page (see crbug.com/666858). Specifically, the +- // RenderViewHost is used to check the "same page" property, while the +- // RenderProcessHost is used to check the "cross-site" property. Note that the +- // reason the RenderProcessHost is tracked instead of the RenderWidgetHost is +- // so that we still allow drags between non-contiguous same-site frames (such +- // frames will have the same process, but different widgets). Note also that +- // the RenderViewHost may not be in the same process as the RenderProcessHost, +- // since the view corresponds to the page, while the process is specific to +- // the frame from which the drag started. +- // We also track whether a dragged image is accessible from its frame, so we +- // can disallow tainted-cross-origin same-page drag-drop. ++ // Used to track security-salient details about a drag source. See ++ // documentation in `IsValidDragTarget()` for `site_instance_group_id`. ++ // See crbug.com/1264873 for `image_accessible_from_frame`. + struct DragStart { + DragStart(SiteInstanceGroupId site_instance_group_id, +- GlobalRoutingID view_id, + bool image_accessible_from_frame) + : site_instance_group_id(site_instance_group_id), +- view_id(view_id), + image_accessible_from_frame(image_accessible_from_frame) {} + ~DragStart() = default; + + SiteInstanceGroupId site_instance_group_id; +- GlobalRoutingID view_id; + bool image_accessible_from_frame; + }; ++ // Will hold a value when the current drag started in this page (outermost ++ // WebContents). + absl::optional drag_start_; + + // Responsible for handling gesture-nav and pull-to-refresh UI.