diff --git a/www/qt6-webengine/Makefile b/www/qt6-webengine/Makefile index b62f3f3a255b..9cf2441c7458 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?= 4 # Master port for print/qt6-pdf. Please keep this line. +PORTREVISION?= 5 # 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 3f67e42ad06b..2f8615470498 100644 --- a/www/qt6-webengine/files/patch-security-rollup +++ b/www/qt6-webengine/files/patch-security-rollup @@ -1,4439 +1,4979 @@ 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 - CVE-2023-6702 - CVE-2023-6703 - CVE-2023-6705 - CVE-2023-6706 - Security bug 1506726 - Security bug 1505632 - Security bug 1488199 - CVE-2023-7024 - CVE-2024-0333 - CVE-2024-0225 - CVE-2024-0224 - CVE-2024-0223 - CVE-2024-0222 - Security bug 1511689 - CVE-2024-0519 - CVE-2024-0518 - Security bug 1506535 - CVE-2024-0808 - CVE-2024-0807 - Security bug 1511389 - CVE-2024-0810 - Security bug 1407197 +- Security bug 1519980 +- CVE-2024-1060 +- CVE-2024-1077 +- CVE-2024-1059 +- CVE-2024-1283 +- CVE-2024-1284 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. From fe07848de4cd69e57f79528a70c75075ca2951dc Mon Sep 17 00:00:00 2001 From: Zakhar Voit Date: Thu, 14 Dec 2023 11:11:43 +0000 Subject: [PATCH] [Backport] CVE-2023-6702: Type Confusion in V8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/5110982: Fix the case when the closure has run M114 changes: - replace IsNativeContext(*context) by context->IsNativeContext() We were using the closure pointing to NativeContext as a marker that the closure has run, but async stack trace code was confused about it. (cherry picked from commit bde3d360097607f36cd1d17cbe8412b84eae0a7f) Bug: chromium:1501326 Change-Id: I30d438f3b2e3fdd7562ea9a79dde4561ce9b0083 Cr-Original-Commit-Position: refs/heads/main@{#90949} Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5110982 Commit-Queue: Marja Hölttä Auto-Submit: Marja Hölttä Cr-Commit-Position: refs/branch-heads/12.0@{#18} Cr-Branched-From: ed7b4caf1fb8184ad9e24346c84424055d4d430a-refs/heads/12.0.267@{#1} Cr-Branched-From: 210e75b19db4352c9b78dce0bae11c2dc3077df4-refs/heads/main@{#90651} (cherry picked from commit cbd09b2ca928f1fd929ef52e173aa81213e38cb8) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526344 Reviewed-by: Michal Klocek --- chromium/v8/src/execution/isolate.cc | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/chromium/v8/src/execution/isolate.cc b/chromium/v8/src/execution/isolate.cc index 1c6c464dd6f..d0dc49cd754 100644 --- src/3rdparty/chromium/v8/src/execution/isolate.cc.orig +++ src/3rdparty/chromium/v8/src/execution/isolate.cc @@ -974,7 +974,13 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle promise, isolate); builder->AppendPromiseCombinatorFrame(function, combinator); - // Now peak into the Promise.all() resolve element context to + if (context->IsNativeContext()) { + // NativeContext is used as a marker that the closure was already + // called. We can't access the reject element context any more. + return; + } + + // Now peek into the Promise.all() resolve element context to // find the promise capability that's being resolved when all // the concurrent promises resolve. int const index = @@ -993,7 +999,13 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle promise, context->native_context().promise_all_settled(), isolate); builder->AppendPromiseCombinatorFrame(function, combinator); - // Now peak into the Promise.allSettled() resolve element context to + if (context->IsNativeContext()) { + // NativeContext is used as a marker that the closure was already + // called. We can't access the reject element context any more. + return; + } + + // Now peek into the Promise.allSettled() resolve element context to // find the promise capability that's being resolved when all // the concurrent promises resolve. int const index = @@ -1011,7 +1023,13 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle promise, isolate); builder->AppendPromiseCombinatorFrame(function, combinator); - // Now peak into the Promise.any() reject element context to + if (context->IsNativeContext()) { + // NativeContext is used as a marker that the closure was already + // called. We can't access the reject element context any more. + return; + } + + // Now peek into the Promise.any() reject element context to // find the promise capability that's being resolved when any of // the concurrent promises resolve. int const index = PromiseBuiltins::kPromiseAnyRejectElementCapabilitySlot; From e935bf78711c7b8e3938eb3b58d6d61fa7fc2127 Mon Sep 17 00:00:00 2001 From: Paul Semel Date: Wed, 6 Dec 2023 15:52:56 +0000 Subject: [PATCH] [Backport] CVE-2023-6703: Use after free in Blink Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5071252: [M120] ImageBitmapFactory: fix empty context dcheck Approved by: https://bugs.chromium.org/p/chromium/issues/detail?id=1502102#c34 (cherry picked from commit c4d2f15b8f97076c8fd0f9aa5814b94db698b75c) Fixed: 1502102 Change-Id: Ib42d2897d62136ae835561bcf56884b5624060a5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5071252 Commit-Queue: Paul Semel Reviewed-by: Jean-Philippe Gravel Cr-Original-Commit-Position: refs/heads/main@{#1230617} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5088373 Auto-Submit: Arthur Sonzogni Reviewed-by: Paul Semel Cr-Commit-Position: refs/branch-heads/6099@{#1416} Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526345 Reviewed-by: Michal Klocek --- .../modules/canvas/imagebitmap/image_bitmap_factories.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chromium/third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_factories.cc b/chromium/third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_factories.cc index 99feb736a50..3a546a64fe8 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_factories.cc.orig +++ src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_factories.cc @@ -156,7 +156,9 @@ ScriptPromise ImageBitmapFactories::CreateImageBitmapFromBlob( ImageBitmapSource* bitmap_source, absl::optional crop_rect, const ImageBitmapOptions* options) { - DCHECK(script_state->ContextIsValid()); + if (!script_state->ContextIsValid()) { + return ScriptPromise(); + } // imageOrientation: 'from-image' will be used to replace imageOrientation: // 'none'. Adding a deprecation warning when 'none' is called in From d5f5cda70cad9ce87e306a1843cf5970bda04236 Mon Sep 17 00:00:00 2001 From: Guido Urdaneta Date: Fri, 1 Dec 2023 08:19:24 +0000 Subject: [PATCH] [Backport] CVE-2023-6705: Use after free in WebRTC Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5077845: Drop frames received on the wrong task runner It can happen during transfer that a frame is posted from the background media thread to the task runner of the old execution context, which can lead to races and UAF. This CL makes underlying sources drop frames received on the wrong task runner to avoid the problem. Bug: 1505708 Change-Id: I686228d88cb1c48bdf8c0b6bf85edd280a54300a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5077845 Commit-Queue: Guido Urdaneta Reviewed-by: Tony Herre Cr-Commit-Position: refs/heads/main@{#1231802} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526346 Reviewed-by: Michal Klocek --- .../rtc_encoded_audio_underlying_source.cc | 10 +++++++++- .../rtc_encoded_video_underlying_source.cc | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_audio_underlying_source.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_audio_underlying_source.cc index 56926ce628fb..64348f4a273e 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_audio_underlying_source.cc.orig +++ src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_audio_underlying_source.cc @@ -60,7 +60,15 @@ void RTCEncodedAudioUnderlyingSource::Trace(Visitor* visitor) const { void RTCEncodedAudioUnderlyingSource::OnFrameFromSource( std::unique_ptr webrtc_frame) { - DCHECK(task_runner_->BelongsToCurrentThread()); + // It can happen that a frame is posted to the task runner of the old + // execution context during a stream transfer to a new context. + // TODO(https://crbug.com/1506631): Make the state updates related to the + // transfer atomic and turn this into a DCHECK. + if (!task_runner_->BelongsToCurrentThread()) { + DVLOG(1) << "Dropped frame posted to incorrect task runner. This can " + "happen during transfer."; + return; + } // If the source is canceled or there are too many queued frames, // drop the new frame. if (!disconnect_callback_ || !GetExecutionContext()) { diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_source.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_source.cc index 54ca7d1529b1..8fb1d8460e28 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_source.cc.orig +++ src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_source.cc @@ -58,7 +58,15 @@ void RTCEncodedVideoUnderlyingSource::Trace(Visitor* visitor) const { void RTCEncodedVideoUnderlyingSource::OnFrameFromSource( std::unique_ptr webrtc_frame) { - DCHECK(task_runner_->BelongsToCurrentThread()); + // It can happen that a frame is posted to the task runner of the old + // execution context during a stream transfer to a new context. + // TODO(https://crbug.com/1506631): Make the state updates related to the + // transfer atomic and turn this into a DCHECK. + if (!task_runner_->BelongsToCurrentThread()) { + DVLOG(1) << "Dropped frame posted to incorrect task runner. This can " + "happen during transfer."; + return; + } // If the source is canceled or there are too many queued frames, // drop the new frame. if (!disconnect_callback_ || !GetExecutionContext()) { From 9050bef97ea5f15232210e6d1096e9badc04d13e Mon Sep 17 00:00:00 2001 From: Yi Gu Date: Tue, 28 Nov 2023 15:51:40 +0000 Subject: [PATCH] [Backport] CVE-2023-6706: Use after free in FedCM Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5095846: [M114-LTS][FedCM] Check API permission before showing accounts UI M114 merge issues: content/browser/webid/federated_auth_request_impl.h/cc: - The GetApiPermissionStatus() doesn't exist in 114, it uses api_permission_delegate_ directly. The accounts fetch could be delayed for legitimate reasons. A user may be able to disable FedCM API (e.g. via settings or dismissing another FedCM UI on the same RP origin) before the browser receives the accounts response. This patch checks the API permission before showing the accounts UI. Change-Id: Idbbe88912941113ec3f54d7f222845cd774dc897 Bug: 1500921 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064052 Commit-Queue: Yi Gu Cr-Commit-Position: refs/heads/main@{#1229912} (cherry picked from commit 98676a2f66c4b4b802316eef70f4aab77e631f85) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526347 Reviewed-by: Michal Klocek --- .../browser/webid/federated_auth_request_impl.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/chromium/content/browser/webid/federated_auth_request_impl.cc b/chromium/content/browser/webid/federated_auth_request_impl.cc index cbae35e588c..8242b6cc502 100644 --- src/3rdparty/chromium/content/browser/webid/federated_auth_request_impl.cc.orig +++ src/3rdparty/chromium/content/browser/webid/federated_auth_request_impl.cc @@ -867,6 +867,19 @@ void FederatedAuthRequestImpl::MaybeShowAccountsDialog() { return; } + // The accounts fetch could be delayed for legitimate reasons. A user may be + // able to disable FedCM API (e.g. via settings or dismissing another FedCM UI + // on the same RP origin) before the browser receives the accounts response. + // We should exit early without showing any UI. + if (api_permission_delegate_->GetApiPermissionStatus(GetEmbeddingOrigin()) != + FederatedApiPermissionStatus::GRANTED) { + CompleteRequestWithError( + FederatedAuthRequestResult::kErrorDisabledInSettings, + TokenStatus::kDisabledInSettings, + /*should_delay_callback=*/true); + return; + } + // The RenderFrameHost may be alive but not visible in the following // situations: // Situation #1: User switched tabs From 4f5687248f9993cf1dacdc034a203b9e2d61c324 Mon Sep 17 00:00:00 2001 From: Vasiliy Telezhnikov Date: Fri, 1 Dec 2023 17:45:27 +0000 Subject: [PATCH] [Backport] Security bug 1506726 Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5078779: Check for slugs count before deserializing Slugs in DrawSlugOp Count is part of serialized data and while we never serialize values less then 1, it can be any value when coming over IPC, we should check that it's positive before substacting one. Bug: 1506726 Change-Id: I244f50a682f2e852b22ba88f1e9cddddb0fdfcb9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5078779 Reviewed-by: Peng Huang Commit-Queue: Vasiliy Telezhnikov Cr-Commit-Position: refs/heads/main@{#1232013} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526348 Reviewed-by: Michal Klocek --- chromium/cc/paint/paint_op.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/chromium/cc/paint/paint_op.cc b/chromium/cc/paint/paint_op.cc index 225630cb462..6f8c3df3d22 100644 --- src/3rdparty/chromium/cc/paint/paint_op.cc.orig +++ src/3rdparty/chromium/cc/paint/paint_op.cc @@ -971,10 +971,12 @@ PaintOp* DrawSlugOp::Deserialize(PaintOpReader& reader, void* output) { reader.Read(&op->flags); unsigned int count = 0; reader.Read(&count); - reader.Read(&op->slug); - op->extra_slugs.resize(count - 1); - for (auto& extra_slug : op->extra_slugs) { - reader.Read(&extra_slug); + if (count > 0) { + reader.Read(&op->slug); + op->extra_slugs.resize(count - 1); + for (auto& extra_slug : op->extra_slugs) { + reader.Read(&extra_slug); + } } return op; } From 6237d58de03877b9ff5bf5bfad1e0eeb6a4c4b4c Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 29 Nov 2023 17:44:48 +0000 Subject: [PATCH] [Backport] Security bug 1505632 Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5069480: Fix reinit order in ContextProviderCommandBuffer::BindToCurrentSequence See comments for explanation. Bug: 1505632 Change-Id: I0f43821a9708af91303048332e9fae5e100deee5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5069480 Reviewed-by: Saifuddin Hitawala Commit-Queue: Kai Ninomiya Reviewed-by: Brendon Tiszka Cr-Commit-Position: refs/heads/main@{#1230735} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/526349 Reviewed-by: Michal Klocek --- .../gpu/context_provider_command_buffer.cc | 24 +++++++++----- .../cpp/gpu/context_provider_command_buffer.h | 31 ++++++++++++++++--- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.cc b/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.cc index 6637a9f89dc..d29c926a49e 100644 --- src/3rdparty/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.cc.orig +++ src/3rdparty/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.cc @@ -169,13 +169,13 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { } // The transfer buffer is used to serialize Dawn commands - transfer_buffer_ = + auto transfer_buffer = std::make_unique(webgpu_helper.get()); // The WebGPUImplementation exposes the WebGPUInterface, as well as the // gpu::ContextSupport interface. auto webgpu_impl = std::make_unique( - webgpu_helper.get(), transfer_buffer_.get(), command_buffer_.get()); + webgpu_helper.get(), transfer_buffer.get(), command_buffer_.get()); bind_result_ = webgpu_impl->Initialize(memory_limits_); if (bind_result_ != gpu::ContextResult::kSuccess) { DLOG(ERROR) << "Failed to initialize WebGPUImplementation."; @@ -187,8 +187,11 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { std::string unique_context_name = base::StringPrintf("%s-%p", type_name.c_str(), webgpu_impl.get()); + // IMPORTANT: These hold raw_ptrs to each other, so must be set together. + // See note in the header (and keep it up to date if things change). impl_ = webgpu_impl.get(); webgpu_interface_ = std::move(webgpu_impl); + transfer_buffer_ = std::move(transfer_buffer); helper_ = std::move(webgpu_helper); } else if (attributes_.enable_raster_interface && !attributes_.enable_gles2_interface && @@ -206,14 +209,14 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { } // The transfer buffer is used to copy resources between the client // process and the GPU process. - transfer_buffer_ = + auto transfer_buffer = std::make_unique(raster_helper.get()); // The RasterImplementation exposes the RasterInterface, as well as the // gpu::ContextSupport interface. DCHECK(channel_); auto raster_impl = std::make_unique( - raster_helper.get(), transfer_buffer_.get(), + raster_helper.get(), transfer_buffer.get(), attributes_.bind_generates_resource, attributes_.lose_context_when_out_of_memory, command_buffer_.get(), channel_->image_decode_accelerator_proxy()); @@ -230,8 +233,11 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { raster_impl->TraceBeginCHROMIUM("gpu_toplevel", unique_context_name.c_str()); + // IMPORTANT: These hold raw_ptrs to each other, so must be set together. + // See note in the header (and keep it up to date if things change). impl_ = raster_impl.get(); raster_interface_ = std::move(raster_impl); + transfer_buffer_ = std::move(transfer_buffer); helper_ = std::move(raster_helper); } else { // The GLES2 helper writes the command buffer protocol. @@ -246,7 +252,7 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { // The transfer buffer is used to copy resources between the client // process and the GPU process. - transfer_buffer_ = + auto transfer_buffer = std::make_unique(gles2_helper.get()); // The GLES2Implementation exposes the OpenGLES2 API, as well as the @@ -259,13 +265,13 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { // we only use it if grcontext_support was requested. gles2_impl = std::make_unique< skia_bindings::GLES2ImplementationWithGrContextSupport>( - gles2_helper.get(), /*share_group=*/nullptr, transfer_buffer_.get(), + gles2_helper.get(), /*share_group=*/nullptr, transfer_buffer.get(), attributes_.bind_generates_resource, attributes_.lose_context_when_out_of_memory, support_client_side_arrays, command_buffer_.get()); } else { gles2_impl = std::make_unique( - gles2_helper.get(), /*share_group=*/nullptr, transfer_buffer_.get(), + gles2_helper.get(), /*share_group=*/nullptr, transfer_buffer.get(), attributes_.bind_generates_resource, attributes_.lose_context_when_out_of_memory, support_client_side_arrays, command_buffer_.get()); @@ -276,8 +282,11 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { return bind_result_; } + // IMPORTANT: These hold raw_ptrs to each other, so must be set together. + // See note in the header (and keep it up to date if things change). impl_ = gles2_impl.get(); gles2_impl_ = std::move(gles2_impl); + transfer_buffer_ = std::move(transfer_buffer); helper_ = std::move(gles2_helper); } @@ -311,6 +320,7 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentSequence() { switches::kEnableGpuClientTracing)) { // This wraps the real GLES2Implementation and we should always use this // instead when it's present. + // IMPORTANT: This holds a raw_ptr to gles2_impl_. trace_impl_ = std::make_unique( gles2_impl_.get()); gl = trace_impl_.get(); diff --git a/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.h b/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.h index e434c1b4fc4..b25506f3b32 100644 --- src/3rdparty/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.h.orig +++ src/3rdparty/chromium/services/viz/public/cpp/gpu/context_provider_command_buffer.h @@ -162,19 +162,42 @@ class ContextProviderCommandBuffer // associated shared images are destroyed. std::unique_ptr shared_image_interface_; - base::Lock context_lock_; // Referenced by command_buffer_. + ////////////////////////////////////////////////////////////////////////////// + // IMPORTANT NOTE: All of the objects in this block are part of a complex // + // graph of raw pointers (holder or pointee of various raw_ptrs). They are // + // defined in topological order: only later items point to earlier items. // + // - When writing any member, always ensure its pointers to earlier members + // are guaranteed to stay alive. + // - When clearing OR overwriting any member, always ensure objects that + // point to it have already been cleared. + // - The topological order of definitions guarantees that the + // destructors will be called in the correct order (bottom to top). + // - When overwriting multiple members, similarly do so in reverse order. + // + // Please note these comments are likely not to stay perfectly up-to-date. + + base::Lock context_lock_; + // Points to the context_lock_ field of `this`. std::unique_ptr command_buffer_; + + // Points to command_buffer_. std::unique_ptr helper_; + // Points to helper_. std::unique_ptr transfer_buffer_; + // Points to transfer_buffer_, helper_, and command_buffer_. std::unique_ptr gles2_impl_; + // Points to gles2_impl_. std::unique_ptr trace_impl_; + // Points to transfer_buffer_, helper_, and command_buffer_. std::unique_ptr raster_interface_; + // Points to transfer_buffer_, helper_, and command_buffer_. std::unique_ptr webgpu_interface_; + // This is an alias for gles2_impl_, raster_interface_, or webgpu_interface_. + raw_ptr impl_ = nullptr; - // Owned by one of gles2_impl_, raster_interface_, or webgpu_interface_. It - // must be declared last and cleared first. - raw_ptr impl_; + // END IMPORTANT NOTE // + ////////////////////////////////////////////////////////////////////////////// std::unique_ptr gr_context_; #if BUILDFLAG(SKIA_USE_DAWN) From 536f81c3de46e6e1ac2b23ed610f9526bce14fe4 Mon Sep 17 00:00:00 2001 From: pthier Date: Tue, 24 Oct 2023 13:28:22 +0200 Subject: [PATCH] [Backport] Security bug 1488199 (1/3) Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/4971832: [regexp] Fix stack check in native code when interrupt was requested When an interrupt was requested at the time we hit the stack check, the check to ensure we have enough space for local variables was skipped. Bug: chromium:1488199 Change-Id: I95d82fe737420d2ef43c1ace35560cfd5860829b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4971832 Commit-Queue: Patrick Thier Reviewed-by: Jakob Linke Cr-Commit-Position: refs/heads/main@{#90560} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/523712 Reviewed-by: Michal Klocek --- .../regexp/arm/regexp-macro-assembler-arm.cc | 23 +++++++----- .../regexp/arm/regexp-macro-assembler-arm.h | 5 +-- .../arm64/regexp-macro-assembler-arm64.cc | 21 ++++++----- .../arm64/regexp-macro-assembler-arm64.h | 6 ++-- .../ia32/regexp-macro-assembler-ia32.cc | 19 ++++++---- .../regexp/ia32/regexp-macro-assembler-ia32.h | 5 +-- .../v8/src/regexp/regexp-macro-assembler.cc | 5 +-- .../v8/src/regexp/regexp-macro-assembler.h | 2 +- .../regexp/x64/regexp-macro-assembler-x64.cc | 36 +++++++++++-------- .../regexp/x64/regexp-macro-assembler-x64.h | 4 +-- 10 files changed, 78 insertions(+), 48 deletions(-) diff --git a/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.cc b/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.cc index 8aa815db2ac..6b167fe3dc2 100644 --- src/3rdparty/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.cc +++ src/3rdparty/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.cc @@ -754,11 +754,13 @@ Handle RegExpMacroAssemblerARM::GetCode(Handle source) { __ mov(r0, Operand(stack_limit)); __ ldr(r0, MemOperand(r0)); __ sub(r0, sp, r0, SetCC); + Operand extra_space_for_variables(num_registers_ * kSystemPointerSize); + // Handle it if the stack pointer is already below the stack limit. __ b(ls, &stack_limit_hit); // Check if there is room for the variable number of registers above // the stack limit. - __ cmp(r0, Operand(num_registers_ * kSystemPointerSize)); + __ cmp(r0, extra_space_for_variables); __ b(hs, &stack_ok); // Exit with OutOfMemory exception. There is not enough space on the stack // for our working registers. @@ -766,7 +768,7 @@ Handle RegExpMacroAssemblerARM::GetCode(Handle source) { __ jmp(&return_r0); __ bind(&stack_limit_hit); - CallCheckStackGuardState(); + CallCheckStackGuardState(extra_space_for_variables); __ cmp(r0, Operand::Zero()); // If returned value is non-zero, we exit with the returned value as result. __ b(ne, &return_r0); @@ -1158,16 +1160,18 @@ void RegExpMacroAssemblerARM::ClearRegisters(int reg_from, int reg_to) { // Private methods: -void RegExpMacroAssemblerARM::CallCheckStackGuardState() { +void RegExpMacroAssemblerARM::CallCheckStackGuardState(Operand extra_space) { DCHECK(!isolate()->IsGeneratingEmbeddedBuiltins()); DCHECK(!masm_->options().isolate_independent_code); - __ PrepareCallCFunction(3); + __ PrepareCallCFunction(4); + // Extra space for variables to consider in stack check. + __ mov(arg_reg_4, extra_space); // RegExp code frame pointer. - __ mov(r2, frame_pointer()); + __ mov(arg_reg_3, frame_pointer()); // InstructionStream of self. - __ mov(r1, Operand(masm_->CodeObject())); + __ mov(arg_reg_2, Operand(masm_->CodeObject())); // We need to make room for the return address on the stack. int stack_alignment = base::OS::ActivationFrameAlignment(); @@ -1195,7 +1199,6 @@ void RegExpMacroAssemblerARM::CallCheckStackGuardState() { __ mov(code_pointer(), Operand(masm_->CodeObject())); } - // Helper function for reading a value out of a stack frame. template static T& frame_entry(Address re_frame, int frame_offset) { @@ -1210,7 +1213,8 @@ static T* frame_entry_address(Address re_frame, int frame_offset) { int RegExpMacroAssemblerARM::CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame) { + Address re_frame, + uintptr_t extra_space) { InstructionStream re_code = InstructionStream::cast(Object(raw_code)); return NativeRegExpMacroAssembler::CheckStackGuardState( frame_entry(re_frame, kIsolateOffset), @@ -1220,7 +1224,8 @@ int RegExpMacroAssemblerARM::CheckStackGuardState(Address* return_address, return_address, re_code, frame_entry_address
(re_frame, kInputStringOffset), frame_entry_address(re_frame, kInputStartOffset), - frame_entry_address(re_frame, kInputEndOffset)); + frame_entry_address(re_frame, kInputEndOffset), + extra_space); } diff --git a/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.h b/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.h index 44be0d920b6..e8d9f6d76de 100644 --- src/3rdparty/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.h +++ src/3rdparty/chromium/v8/src/regexp/arm/regexp-macro-assembler-arm.h @@ -88,7 +88,7 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerARM // returning. // {raw_code} is an Address because this is called via ExternalReference. static int CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame); + Address re_frame, uintptr_t extra_space); private: // Offsets from frame_pointer() of function parameters and stored registers. @@ -152,7 +152,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerARM // Check whether we are exceeding the stack limit on the backtrack stack. void CheckStackLimit(); - void CallCheckStackGuardState(); + void CallCheckStackGuardState( + Operand extra_space_for_variables = Operand::Zero()); void CallIsCharacterInRangeArray(const ZoneList* ranges); // The ebp-relative location of a regexp register. diff --git a/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc b/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc index d453922f6bf..e8d48236621 100644 --- src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc +++ src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc @@ -866,13 +866,14 @@ Handle RegExpMacroAssemblerARM64::GetCode(Handle source) { __ Mov(x10, stack_limit); __ Ldr(x10, MemOperand(x10)); __ Subs(x10, sp, x10); + Operand extra_space_for_variables(num_wreg_to_allocate * kWRegSize); // Handle it if the stack pointer is already below the stack limit. __ B(ls, &stack_limit_hit); // Check if there is room for the variable number of registers above // the stack limit. - __ Cmp(x10, num_wreg_to_allocate * kWRegSize); + __ Cmp(x10, extra_space_for_variables); __ B(hs, &stack_ok); // Exit with OutOfMemory exception. There is not enough space on the stack @@ -881,7 +882,7 @@ Handle RegExpMacroAssemblerARM64::GetCode(Handle source) { __ B(&return_w0); __ Bind(&stack_limit_hit); - CallCheckStackGuardState(x10); + CallCheckStackGuardState(x10, extra_space_for_variables); // If returned value is non-zero, we exit with the returned value as result. __ Cbnz(w0, &return_w0); @@ -1433,7 +1434,8 @@ static T* frame_entry_address(Address re_frame, int frame_offset) { int RegExpMacroAssemblerARM64::CheckStackGuardState( Address* return_address, Address raw_code, Address re_frame, - int start_index, const byte** input_start, const byte** input_end) { + int start_index, const byte** input_start, const byte** input_end, + uintptr_t extra_space) { InstructionStream re_code = InstructionStream::cast(Object(raw_code)); return NativeRegExpMacroAssembler::CheckStackGuardState( frame_entry(re_frame, kIsolateOffset), start_index, @@ -1441,7 +1443,7 @@ int RegExpMacroAssemblerARM64::CheckStackGuardState( frame_entry(re_frame, kDirectCallOffset)), return_address, re_code, frame_entry_address
(re_frame, kInputStringOffset), input_start, - input_end); + input_end, extra_space); } @@ -1460,21 +1462,24 @@ void RegExpMacroAssemblerARM64::CheckPosition(int cp_offset, // Private methods: -void RegExpMacroAssemblerARM64::CallCheckStackGuardState(Register scratch) { +void RegExpMacroAssemblerARM64::CallCheckStackGuardState(Register scratch, + Operand extra_space) { DCHECK(!isolate()->IsGeneratingEmbeddedBuiltins()); DCHECK(!masm_->options().isolate_independent_code); // Allocate space on the stack to store the return address. The // CheckStackGuardState C++ function will override it if the code - // moved. Allocate extra space for 2 arguments passed by pointers. - // AAPCS64 requires the stack to be 16 byte aligned. + // moved. Allocate extra space for 3 arguments (2 for input start/end and 1 + // for gap). AAPCS64 requires the stack to be 16 byte aligned. int alignment = masm_->ActivationFrameAlignment(); DCHECK_EQ(alignment % 16, 0); int align_mask = (alignment / kXRegSize) - 1; - int xreg_to_claim = (3 + align_mask) & ~align_mask; + int xreg_to_claim = (4 + align_mask) & ~align_mask; __ Claim(xreg_to_claim); + __ Mov(x0, extra_space); + __ Poke(x0, 3 * kSystemPointerSize); // CheckStackGuardState needs the end and start addresses of the input string. __ Poke(input_end(), 2 * kSystemPointerSize); __ Add(x5, sp, 2 * kSystemPointerSize); diff --git a/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.h b/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.h index a5164472b71..05b4eb5bd7b 100644 --- src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.h +++ src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.h @@ -95,7 +95,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerARM64 static int CheckStackGuardState(Address* return_address, Address raw_code, Address re_frame, int start_offset, const byte** input_start, - const byte** input_end); + const byte** input_end, + uintptr_t extra_space); private: static constexpr int kFramePointerOffset = 0; @@ -174,7 +175,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerARM64 // Check whether we are exceeding the stack limit on the backtrack stack. void CheckStackLimit(); - void CallCheckStackGuardState(Register scratch); + void CallCheckStackGuardState(Register scratch, + Operand extra_space = Operand(0)); void CallIsCharacterInRangeArray(const ZoneList* ranges); // Location of a 32 bit position register. diff --git a/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.cc b/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.cc index 6370e7e57da..8dff4abcb25 100644 --- src/3rdparty/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.cc +++ src/3rdparty/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.cc @@ -801,11 +801,13 @@ Handle RegExpMacroAssemblerIA32::GetCode(Handle source) { ExternalReference::address_of_jslimit(isolate()); __ mov(eax, esp); __ sub(eax, StaticVariable(stack_limit)); + Immediate extra_space_for_variables(num_registers_ * kSystemPointerSize); + // Handle it if the stack pointer is already below the stack limit. __ j(below_equal, &stack_limit_hit); // Check if there is room for the variable number of registers above // the stack limit. - __ cmp(eax, num_registers_ * kSystemPointerSize); + __ cmp(eax, extra_space_for_variables); __ j(above_equal, &stack_ok); // Exit with OutOfMemory exception. There is not enough space on the stack // for our working registers. @@ -814,7 +816,7 @@ Handle RegExpMacroAssemblerIA32::GetCode(Handle source) { __ bind(&stack_limit_hit); __ push(backtrack_stackpointer()); - CallCheckStackGuardState(ebx); + CallCheckStackGuardState(ebx, extra_space_for_variables); __ pop(backtrack_stackpointer()); __ or_(eax, eax); // If returned value is non-zero, we exit with the returned value as result. @@ -1214,9 +1216,12 @@ void RegExpMacroAssemblerIA32::ClearRegisters(int reg_from, int reg_to) { // Private methods: -void RegExpMacroAssemblerIA32::CallCheckStackGuardState(Register scratch) { - static const int num_arguments = 3; +void RegExpMacroAssemblerIA32::CallCheckStackGuardState(Register scratch, + Immediate extra_space) { + static const int num_arguments = 4; __ PrepareCallCFunction(num_arguments, scratch); + // Extra space for variables. + __ mov(Operand(esp, 3 * kSystemPointerSize), extra_space); // RegExp code frame pointer. __ mov(Operand(esp, 2 * kSystemPointerSize), ebp); // InstructionStream of self. @@ -1247,7 +1252,8 @@ static T* frame_entry_address(Address re_frame, int frame_offset) { int RegExpMacroAssemblerIA32::CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame) { + Address re_frame, + uintptr_t extra_space) { InstructionStream re_code = InstructionStream::cast(Object(raw_code)); return NativeRegExpMacroAssembler::CheckStackGuardState( frame_entry(re_frame, kIsolateOffset), @@ -1257,7 +1263,8 @@ int RegExpMacroAssemblerIA32::CheckStackGuardState(Address* return_address, return_address, re_code, frame_entry_address
(re_frame, kInputStringOffset), frame_entry_address(re_frame, kInputStartOffset), - frame_entry_address(re_frame, kInputEndOffset)); + frame_entry_address(re_frame, kInputEndOffset), + extra_space); } diff --git a/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.h b/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.h index 649c61d880e..a33b687c8c3 100644 --- src/3rdparty/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.h +++ src/3rdparty/chromium/v8/src/regexp/ia32/regexp-macro-assembler-ia32.h @@ -89,7 +89,7 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerIA32 // returning. // {raw_code} is an Address because this is called via ExternalReference. static int CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame); + Address re_frame, uintptr_t extra_space); private: Operand StaticVariable(const ExternalReference& ext); @@ -159,7 +159,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerIA32 // Check whether we are exceeding the stack limit on the backtrack stack. void CheckStackLimit(); - void CallCheckStackGuardState(Register scratch); + void CallCheckStackGuardState(Register scratch, + Immediate extra_space = Immediate(0)); void CallIsCharacterInRangeArray(const ZoneList* ranges); // The ebp-relative location of a regexp register. diff --git a/chromium/v8/src/regexp/regexp-macro-assembler.cc b/chromium/v8/src/regexp/regexp-macro-assembler.cc index 2fcb0a425ec..19c4cc9c8dd 100644 --- src/3rdparty/chromium/v8/src/regexp/regexp-macro-assembler.cc +++ src/3rdparty/chromium/v8/src/regexp/regexp-macro-assembler.cc @@ -284,14 +284,15 @@ bool NativeRegExpMacroAssembler::CanReadUnaligned() const { int NativeRegExpMacroAssembler::CheckStackGuardState( Isolate* isolate, int start_index, RegExp::CallOrigin call_origin, Address* return_address, InstructionStream re_code, Address* subject, - const byte** input_start, const byte** input_end) { + const byte** input_start, const byte** input_end, + uintptr_t gap) { DisallowGarbageCollection no_gc; Address old_pc = PointerAuthentication::AuthenticatePC(return_address, 0); DCHECK_LE(re_code.instruction_start(), old_pc); DCHECK_LE(old_pc, re_code.instruction_end()); StackLimitCheck check(isolate); - bool js_has_overflowed = check.JsHasOverflowed(); + bool js_has_overflowed = check.JsHasOverflowed(gap); if (call_origin == RegExp::CallOrigin::kFromJs) { // Direct calls from JavaScript can be interrupted in two ways: diff --git a/chromium/v8/src/regexp/regexp-macro-assembler.h b/chromium/v8/src/regexp/regexp-macro-assembler.h index 2ba9e2d28d0..4d16f8a98ad 100644 --- src/3rdparty/chromium/v8/src/regexp/regexp-macro-assembler.h +++ src/3rdparty/chromium/v8/src/regexp/regexp-macro-assembler.h @@ -334,7 +334,7 @@ class NativeRegExpMacroAssembler: public RegExpMacroAssembler { Address* return_address, InstructionStream re_code, Address* subject, const byte** input_start, - const byte** input_end); + const byte** input_end, uintptr_t gap); static Address word_character_map_address() { return reinterpret_cast
(&word_character_map[0]); diff --git a/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.cc b/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.cc index b6b2f5f5606..83f5cbf2d29 100644 --- src/3rdparty/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.cc +++ src/3rdparty/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.cc @@ -842,11 +842,13 @@ Handle RegExpMacroAssemblerX64::GetCode(Handle source) { __ movq(r9, rsp); __ Move(kScratchRegister, stack_limit); __ subq(r9, Operand(kScratchRegister, 0)); + Immediate extra_space_for_variables(num_registers_ * kSystemPointerSize); + // Handle it if the stack pointer is already below the stack limit. __ j(below_equal, &stack_limit_hit); // Check if there is room for the variable number of registers above // the stack limit. - __ cmpq(r9, Immediate(num_registers_ * kSystemPointerSize)); + __ cmpq(r9, extra_space_for_variables); __ j(above_equal, &stack_ok); // Exit with OutOfMemory exception. There is not enough space on the stack // for our working registers. @@ -856,7 +858,8 @@ Handle RegExpMacroAssemblerX64::GetCode(Handle source) { __ bind(&stack_limit_hit); __ Move(code_object_pointer(), masm_.CodeObject()); __ pushq(backtrack_stackpointer()); - CallCheckStackGuardState(); // Preserves no registers beside rbp and rsp. + // CallCheckStackGuardState preserves no registers beside rbp and rsp. + CallCheckStackGuardState(extra_space_for_variables); __ popq(backtrack_stackpointer()); __ testq(rax, rax); // If returned value is non-zero, we exit with the returned value as result. @@ -1267,35 +1270,38 @@ void RegExpMacroAssemblerX64::ClearRegisters(int reg_from, int reg_to) { // Private methods: -void RegExpMacroAssemblerX64::CallCheckStackGuardState() { +void RegExpMacroAssemblerX64::CallCheckStackGuardState(Immediate extra_space) { // This function call preserves no register values. Caller should // store anything volatile in a C call or overwritten by this function. - static const int num_arguments = 3; + static const int num_arguments = 4; __ PrepareCallCFunction(num_arguments); #ifdef V8_TARGET_OS_WIN + // Fourth argument: Extra space for variables. + __ movq(arg_reg_4, extra_space); // Second argument: InstructionStream of self. (Do this before overwriting - // r8). - __ movq(rdx, code_object_pointer()); + // r8 (arg_reg_3)). + __ movq(arg_reg_2, code_object_pointer()); // Third argument: RegExp code frame pointer. - __ movq(r8, rbp); + __ movq(arg_reg_3, rbp); // First argument: Next address on the stack (will be address of // return address). - __ leaq(rcx, Operand(rsp, -kSystemPointerSize)); + __ leaq(arg_reg_1, Operand(rsp, -kSystemPointerSize)); #else + // Fourth argument: Extra space for variables. + __ movq(arg_reg_4, extra_space); // Third argument: RegExp code frame pointer. - __ movq(rdx, rbp); + __ movq(arg_reg_3, rbp); // Second argument: InstructionStream of self. - __ movq(rsi, code_object_pointer()); + __ movq(arg_reg_2, code_object_pointer()); // First argument: Next address on the stack (will be address of // return address). - __ leaq(rdi, Operand(rsp, -kSystemPointerSize)); + __ leaq(arg_reg_1, Operand(rsp, -kSystemPointerSize)); #endif ExternalReference stack_check = ExternalReference::re_check_stack_guard_state(); CallCFunctionFromIrregexpCode(stack_check, num_arguments); } - // Helper function for reading a value out of a stack frame. template static T& frame_entry(Address re_frame, int frame_offset) { @@ -1310,7 +1316,8 @@ static T* frame_entry_address(Address re_frame, int frame_offset) { int RegExpMacroAssemblerX64::CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame) { + Address re_frame, + uintptr_t extra_space) { InstructionStream re_code = InstructionStream::cast(Object(raw_code)); return NativeRegExpMacroAssembler::CheckStackGuardState( frame_entry(re_frame, kIsolateOffset), @@ -1320,7 +1327,8 @@ int RegExpMacroAssemblerX64::CheckStackGuardState(Address* return_address, return_address, re_code, frame_entry_address
(re_frame, kInputStringOffset), frame_entry_address(re_frame, kInputStartOffset), - frame_entry_address(re_frame, kInputEndOffset)); + frame_entry_address(re_frame, kInputEndOffset), + extra_space); } diff --git a/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.h b/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.h index bfe8290a19c..85dacfddf6a 100644 --- src/3rdparty/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.h +++ src/3rdparty/chromium/v8/src/regexp/x64/regexp-macro-assembler-x64.h @@ -88,7 +88,7 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerX64 // returning. // {raw_code} is an Address because this is called via ExternalReference. static int CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame); + Address re_frame, uintptr_t extra_space); private: // Offsets from rbp of function parameters and stored registers. @@ -198,7 +198,7 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerX64 // Check whether we are exceeding the stack limit on the backtrack stack. void CheckStackLimit(); - void CallCheckStackGuardState(); + void CallCheckStackGuardState(Immediate extra_space = Immediate(0)); void CallIsCharacterInRangeArray(const ZoneList* ranges); // The rbp-relative location of a regexp register. From b2748c8718a07d0bfbfcdde10866c43c1708676e Mon Sep 17 00:00:00 2001 From: Lu Yahan Date: Wed, 25 Oct 2023 10:44:00 +0800 Subject: [PATCH] [Backport] Security bug 1488199 (2/3) Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/4975715: [riscv][regexp] Fix stack check in native code when interrupt was requested Port commit 7f1aaf2a1c763c8aa1358ec8ed24f39bfa17b767 Bug: chromium:1488199 Change-Id: I6b2567267cee0b30230b2e42b8606188011b4463 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4975715 Auto-Submit: Yahan Lu Commit-Queue: Yahan Lu Commit-Queue: Ji Qiu Reviewed-by: Ji Qiu Cr-Commit-Position: refs/heads/main@{#90597} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/523713 Reviewed-by: Michal Klocek --- .../regexp/riscv/regexp-macro-assembler-riscv.cc | 15 ++++++++++----- .../regexp/riscv/regexp-macro-assembler-riscv.h | 5 +++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.cc b/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.cc index 5bf630d6200..30337c75978 100644 --- src/3rdparty/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.cc +++ src/3rdparty/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.cc @@ -732,18 +732,19 @@ Handle RegExpMacroAssemblerRISCV::GetCode(Handle source) { __ li(a0, Operand(stack_limit)); __ LoadWord(a0, MemOperand(a0)); __ SubWord(a0, sp, a0); + Operand extra_space_for_variables(num_registers_ * kSystemPointerSize); // Handle it if the stack pointer is already below the stack limit. __ Branch(&stack_limit_hit, le, a0, Operand(zero_reg)); // Check if there is room for the variable number of registers above // the stack limit. - __ Branch(&stack_ok, uge, a0, Operand(num_registers_ * kPointerSize)); + __ Branch(&stack_ok, uge, a0, extra_space_for_variables); // Exit with OutOfMemory exception. There is not enough space on the stack // for our working registers. __ li(a0, Operand(EXCEPTION)); __ jmp(&return_a0); __ bind(&stack_limit_hit); - CallCheckStackGuardState(a0); + CallCheckStackGuardState(a0, extra_space_for_variables); // If returned value is non-zero, we exit with the returned value as // result. __ Branch(&return_a0, ne, a0, Operand(zero_reg)); @@ -1142,7 +1143,8 @@ bool RegExpMacroAssemblerRISCV::CanReadUnaligned() const { return false; } #endif // Private methods: -void RegExpMacroAssemblerRISCV::CallCheckStackGuardState(Register scratch) { +void RegExpMacroAssemblerRISCV::CallCheckStackGuardState(Register scratch, + Operand extra_space) { DCHECK(!isolate()->IsGeneratingEmbeddedBuiltins()); DCHECK(!masm_->options().isolate_independent_code); @@ -1155,6 +1157,7 @@ void RegExpMacroAssemblerRISCV::CallCheckStackGuardState(Register scratch) { __ And(sp, sp, Operand(-stack_alignment)); __ StoreWord(scratch, MemOperand(sp)); + __ li(a3, extra_space); __ mv(a2, frame_pointer()); // InstructionStream of self. __ li(a1, Operand(masm_->CodeObject()), CONSTANT_SIZE); @@ -1215,7 +1218,8 @@ static T* frame_entry_address(Address re_frame, int frame_offset) { int64_t RegExpMacroAssemblerRISCV::CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame) { + Address re_frame, + uintptr_t extra_space) { InstructionStream re_code = InstructionStream::cast(Object(raw_code)); return NativeRegExpMacroAssembler::CheckStackGuardState( frame_entry(re_frame, kIsolateOffset), @@ -1225,7 +1229,8 @@ int64_t RegExpMacroAssemblerRISCV::CheckStackGuardState(Address* return_address, return_address, re_code, frame_entry_address
(re_frame, kInputStringOffset), frame_entry_address(re_frame, kInputStartOffset), - frame_entry_address(re_frame, kInputEndOffset)); + frame_entry_address(re_frame, kInputEndOffset), + extra_space); } MemOperand RegExpMacroAssemblerRISCV::register_location(int register_index) { diff --git a/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.h b/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.h index 90a1d314cc7..aa45a531b85 100644 --- src/3rdparty/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.h +++ src/3rdparty/chromium/v8/src/regexp/riscv/regexp-macro-assembler-riscv.h @@ -91,7 +91,7 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerRISCV // returning. // {raw_code} is an Address because this is called via ExternalReference. static int64_t CheckStackGuardState(Address* return_address, Address raw_code, - Address re_frame); + Address re_frame, uintptr_t extra_space); void print_regexp_frame_constants(); @@ -165,7 +165,8 @@ class V8_EXPORT_PRIVATE RegExpMacroAssemblerRISCV // Check whether we are exceeding the stack limit on the backtrack stack. void CheckStackLimit(); - void CallCheckStackGuardState(Register scratch); + void CallCheckStackGuardState(Register scratch, + Operand extra_space_for_variables = Operand(0)); void CallIsCharacterInRangeArray(const ZoneList* ranges); // The ebp-relative location of a regexp register. From c660893ad341e4d5a81ddaf8b23dadcb6cd51660 Mon Sep 17 00:00:00 2001 From: pthier Date: Mon, 30 Oct 2023 11:59:09 +0100 Subject: [PATCH] [Backport] Security bug 1488199 (3/3) Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/4987306: [regexp][arm64] Fix stack check extra space argument Pass argument in register instead of the stack. Bug: chromium:1488199, v8:14415 Change-Id: Ic9967c9f2ca5da1981a0138ddb5f0335ab7f1425 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4987306 Commit-Queue: Patrick Thier Reviewed-by: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#90669} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/523714 Reviewed-by: Michal Klocek --- .../v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc b/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc index e8d48236621..dd3f047ffc4 100644 --- src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc +++ src/3rdparty/chromium/v8/src/regexp/arm64/regexp-macro-assembler-arm64.cc @@ -1469,17 +1469,16 @@ void RegExpMacroAssemblerARM64::CallCheckStackGuardState(Register scratch, // Allocate space on the stack to store the return address. The // CheckStackGuardState C++ function will override it if the code - // moved. Allocate extra space for 3 arguments (2 for input start/end and 1 - // for gap). AAPCS64 requires the stack to be 16 byte aligned. + // moved. Allocate extra space for 2 arguments passed by pointers. + // AAPCS64 requires the stack to be 16 byte aligned. int alignment = masm_->ActivationFrameAlignment(); DCHECK_EQ(alignment % 16, 0); int align_mask = (alignment / kXRegSize) - 1; - int xreg_to_claim = (4 + align_mask) & ~align_mask; + int xreg_to_claim = (3 + align_mask) & ~align_mask; __ Claim(xreg_to_claim); - __ Mov(x0, extra_space); - __ Poke(x0, 3 * kSystemPointerSize); + __ Mov(x6, extra_space); // CheckStackGuardState needs the end and start addresses of the input string. __ Poke(input_end(), 2 * kSystemPointerSize); __ Add(x5, sp, 2 * kSystemPointerSize); From 243fab4932e66121061ada4bb4724ed341c6622e Mon Sep 17 00:00:00 2001 From: Gustaf Ullberg Date: Tue, 19 Dec 2023 18:08:19 +0000 Subject: [PATCH] [Backport] CVE-2023-7024: Heap buffer overflow in WebRTC Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5136295: WebRtcAudioSink: Stop on invalid configuration Bug: 1513170 Change-Id: Ia4ca55e9eafb81789b28b8b8c54e615ac28df633 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5136295 Reviewed-by: Harald Alvestrand Commit-Queue: Gustaf Ullberg Cr-Commit-Position: refs/heads/main@{#1239233} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530064 Reviewed-by: Michal Klocek --- .../blink/renderer/platform/peerconnection/webrtc_audio_sink.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chromium/third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.cc b/chromium/third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.cc index cd9f2edbf6e..209a2277056 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.cc +++ src/3rdparty/chromium/third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.cc @@ -121,7 +121,7 @@ void WebRtcAudioSink::OnData(const media::AudioBus& audio_bus, } void WebRtcAudioSink::OnSetFormat(const media::AudioParameters& params) { - DCHECK(params.IsValid()); + CHECK(params.IsValid()); SendLogMessage(base::StringPrintf("OnSetFormat([label=%s] {params=[%s]})", adapter_->label().c_str(), params.AsHumanReadableString().c_str())); From 6027a6d13bd29c144d3340d997dac822f625086e Mon Sep 17 00:00:00 2001 From: Joshua Pawlicki Date: Wed, 20 Dec 2023 22:33:06 +0000 Subject: [PATCH] [Backport] CVE-2024-0333: Insufficient data validation in Extensions Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5141787: crx_file: Error early for CRXs with ZIP markers in header. Bug: 1513379 Change-Id: I029b4f15778df0c150866b1f49a9b5b2924690ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5141787 Commit-Queue: Joshua Pawlicki Auto-Submit: Joshua Pawlicki Code-Coverage: findit-for-me@appspot.gserviceaccount.com Commit-Queue: Sorin Jianu Reviewed-by: Sorin Jianu Cr-Commit-Position: refs/heads/main@{#1239849} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530065 Reviewed-by: Michal Klocek --- chromium/components/crx_file/crx_verifier.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/chromium/components/crx_file/crx_verifier.cc b/chromium/components/crx_file/crx_verifier.cc index 2378aa5a951..cf038c50bdb 100644 --- src/3rdparty/chromium/components/crx_file/crx_verifier.cc +++ src/3rdparty/chromium/components/crx_file/crx_verifier.cc @@ -4,6 +4,7 @@ #include "components/crx_file/crx_verifier.h" +#include #include #include #include @@ -43,6 +44,9 @@ constexpr uint8_t kPublisherTestKeyHash[] = { 0x5f, 0x64, 0xf3, 0xa6, 0x17, 0x03, 0x0d, 0xde, 0x21, 0x61, 0xbe, 0xb7, 0x95, 0x91, 0x95, 0x83, 0x68, 0x12, 0xe9, 0x78, 0x1e}; +constexpr uint8_t kEocd[] = {'P', 'K', 0x05, 0x06}; +constexpr uint8_t kEocd64[] = {'P', 'K', 0x06, 0x07}; + using VerifierCollection = std::vector>; using RepeatedProof = google::protobuf::RepeatedPtrField; @@ -109,6 +113,18 @@ VerifierResult VerifyCrx3( header_size) { return VerifierResult::ERROR_HEADER_INVALID; } + + // If the header contains a ZIP EOCD or EOCD64 token, unzipping may not work + // correctly. + if (std::search(std::begin(header_bytes), std::end(header_bytes), + std::begin(kEocd), + std::end(kEocd)) != std::end(header_bytes) || + std::search(std::begin(header_bytes), std::end(header_bytes), + std::begin(kEocd64), + std::end(kEocd64)) != std::end(header_bytes)) { + return VerifierResult::ERROR_HEADER_INVALID; + } + CrxFileHeader header; if (!header.ParseFromArray(header_bytes.data(), header_size)) return VerifierResult::ERROR_HEADER_INVALID; From 70667dfeaa0cac885821a5fd1479502ff02c78ae Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Tue, 19 Dec 2023 17:25:51 +0000 Subject: [PATCH] [Backport] CVE-2024-0225: Use after free in WebGPU Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5133239: Use cross thread handles to bind args for async webgpu context creation (cherry picked from commit 542b278a0c1de7202f4bf5e3e5cbdc2dd6c337d4) Fixed: 1506923 Change-Id: I174703cbd993471e3afb39c0cfa4cce2770755f7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5113019 Reviewed-by: Corentin Wallez Commit-Queue: Austin Eng Reviewed-by: Stephen White Cr-Original-Commit-Position: refs/heads/main@{#1237179} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5133239 Cr-Commit-Position: refs/branch-heads/6099@{#1551} Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530066 Reviewed-by: Michal Klocek --- .../blink/renderer/modules/webgpu/gpu.cc | 19 ++++++++++++++++--- .../web_graphics_context_3d_provider_util.cc | 7 +++---- .../web_graphics_context_3d_provider_util.h | 5 +++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/chromium/third_party/blink/renderer/modules/webgpu/gpu.cc b/chromium/third_party/blink/renderer/modules/webgpu/gpu.cc index df95fe1d397..273c874affb 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/webgpu/gpu.cc +++ src/3rdparty/chromium/third_party/blink/renderer/modules/webgpu/gpu.cc @@ -35,11 +35,13 @@ #include "third_party/blink/renderer/platform/graphics/gpu/dawn_control_client_holder.h" #include "third_party/blink/renderer/platform/graphics/gpu/webgpu_callback.h" #include "third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h" +#include "third_party/blink/renderer/platform/heap/cross_thread_handle.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h" #include "third_party/blink/renderer/platform/heap/thread_state.h" #include "third_party/blink/renderer/platform/instrumentation/use_counter.h" #include "third_party/blink/renderer/platform/privacy_budget/identifiability_digest_helpers.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" +#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h" namespace blink { @@ -288,9 +290,19 @@ void GPU::RequestAdapterImpl(ScriptState* script_state, CreateWebGPUGraphicsContext3DProviderAsync( execution_context->Url(), execution_context->GetTaskRunner(TaskType::kWebGPU), - WTF::BindOnce( - [](GPU* gpu, ExecutionContext* execution_context, + CrossThreadBindOnce( + [](CrossThreadHandle gpu_handle, + CrossThreadHandle execution_context_handle, std::unique_ptr context_provider) { + auto unwrap_gpu = MakeUnwrappingCrossThreadHandle(gpu_handle); + auto unwrap_execution_context = + MakeUnwrappingCrossThreadHandle(execution_context_handle); + if (!unwrap_gpu || !unwrap_execution_context) { + return; + } + auto* gpu = unwrap_gpu.GetOnCreationThread(); + auto* execution_context = + unwrap_execution_context.GetOnCreationThread(); const KURL& url = execution_context->Url(); context_provider = CheckContextProvider(url, std::move(context_provider)); @@ -312,7 +324,8 @@ void GPU::RequestAdapterImpl(ScriptState* script_state, std::move(callback).Run(); } }, - WrapPersistent(this), WrapPersistent(execution_context))); + MakeCrossThreadHandle(this), + MakeCrossThreadHandle(execution_context))); return; } diff --git a/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc b/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc index f859f3e62c5..3d9890b9b4a 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc +++ src/3rdparty/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc @@ -121,8 +121,8 @@ CreateWebGPUGraphicsContext3DProvider(const KURL& url) { void CreateWebGPUGraphicsContext3DProviderAsync( const KURL& url, scoped_refptr current_thread_task_runner, - base::OnceCallback)> - callback) { + WTF::CrossThreadOnceFunction< + void(std::unique_ptr)> callback) { if (IsMainThread()) { std::move(callback).Run( Platform::Current()->CreateWebGPUGraphicsContext3DProvider(url)); @@ -140,8 +140,7 @@ void CreateWebGPUGraphicsContext3DProviderAsync( AccessMainThreadForWebGraphicsContext3DProvider()), FROM_HERE, CrossThreadBindOnce(&CreateWebGPUGraphicsContextOnMainThreadAsync, url, - current_thread_task_runner, - CrossThreadBindOnce(std::move(callback)))); + current_thread_task_runner, std::move(callback))); } } diff --git a/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h b/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h index 8fcab24bfec..8b785cc30ac 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h +++ src/3rdparty/chromium/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h @@ -10,6 +10,7 @@ #include "third_party/blink/public/platform/web_graphics_context_3d_provider.h" #include "third_party/blink/renderer/platform/platform_export.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" +#include "third_party/blink/renderer/platform/wtf/functional.h" namespace blink { @@ -42,8 +43,8 @@ CreateWebGPUGraphicsContext3DProvider(const KURL& url); PLATFORM_EXPORT void CreateWebGPUGraphicsContext3DProviderAsync( const KURL& url, scoped_refptr current_thread_task_runner, - base::OnceCallback)> - callback); + WTF::CrossThreadOnceFunction< + void(std::unique_ptr)> callback); } // namespace blink From a3d7e657936027aa3f3a257d3afd525c81c152f0 Mon Sep 17 00:00:00 2001 From: Hongchan Choi Date: Tue, 12 Dec 2023 02:36:08 +0000 Subject: [PATCH] [Backport] CVE-2024-0224: Use after free in WebAudio Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5112992: Wrap buffer read index in delay kernel The current code assumes that the first buffer read index in the delay kernel does not go out of bound. This CL applies the wrapping function to the read index array. (cherry picked from commit fb96fd5f41bec823dbb208d9a7d53fbbf4d16ce4) Bug: 1505086 Test: Locally confirmed the repro does not crash anymore Change-Id: Idca3dfc7dec5b5a7f9b22d87135e2d775729631a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5072113 Commit-Queue: Hongchan Choi Reviewed-by: Michael Wilson Cr-Original-Commit-Position: refs/heads/main@{#1231040} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5112992 Auto-Submit: Hongchan Choi Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/6099@{#1498} Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530067 Reviewed-by: Michal Klocek --- .../renderer/platform/audio/audio_delay_dsp_kernel.cc | 2 +- .../audio/cpu/arm/audio_delay_dsp_kernel_neon.cc | 7 +++++-- .../audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc | 10 +++++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/chromium/third_party/blink/renderer/platform/audio/audio_delay_dsp_kernel.cc b/chromium/third_party/blink/renderer/platform/audio/audio_delay_dsp_kernel.cc index b6613ff3df7b..2e17efd95802 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/audio/audio_delay_dsp_kernel.cc +++ src/3rdparty/chromium/third_party/blink/renderer/platform/audio/audio_delay_dsp_kernel.cc @@ -155,7 +155,7 @@ int AudioDelayDSPKernel::ProcessARateScalar(unsigned start, const float* delay_times = delay_times_.Data(); for (unsigned i = start; i < frames_to_process; ++i) { - double delay_time = delay_times[i]; + double delay_time = std::fmax(delay_times[i], 0); double desired_delay_frames = delay_time * sample_rate; double read_position = w_index + buffer_length - desired_delay_frames; diff --git a/chromium/third_party/blink/renderer/platform/audio/cpu/arm/audio_delay_dsp_kernel_neon.cc b/chromium/third_party/blink/renderer/platform/audio/cpu/arm/audio_delay_dsp_kernel_neon.cc index e3c6fd5eb064..8bf0d2a57305 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/audio/cpu/arm/audio_delay_dsp_kernel_neon.cc +++ src/3rdparty/chromium/third_party/blink/renderer/platform/audio/cpu/arm/audio_delay_dsp_kernel_neon.cc @@ -60,6 +60,7 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( int w_index = write_index_; const float32x4_t v_sample_rate = vdupq_n_f32(sample_rate); + const float32x4_t v_all_zeros = vdupq_n_f32(0); // The buffer length as a float and as an int so we don't need to constant // convert from one to the other. @@ -87,7 +88,8 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( int k = 0; for (int n = 0; n < number_of_loops; ++n, k += 4) { - const float32x4_t v_delay_time = vld1q_f32(delay_times + k); + const float32x4_t v_delay_time = vmaxq_f32(vld1q_f32(delay_times + k), + v_all_zeros); const float32x4_t v_desired_delay_frames = vmulq_f32(v_delay_time, v_sample_rate); @@ -100,7 +102,8 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( WrapPositionVector(v_read_position, v_buffer_length_float); // Get indices into the buffer for the samples we need for interpolation. - const int32x4_t v_read_index1 = vcvtq_s32_f32(v_read_position); + const int32x4_t v_read_index1 = WrapIndexVector( + vcvtq_s32_f32(v_read_position), v_buffer_length_int); const int32x4_t v_read_index2 = WrapIndexVector( vaddq_s32(v_read_index1, vdupq_n_s32(1)), v_buffer_length_int); diff --git a/chromium/third_party/blink/renderer/platform/audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc b/chromium/third_party/blink/renderer/platform/audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc index fc409b192d62..c2443da23e55 100644 --- src/3rdparty/chromium/third_party/blink/renderer/platform/audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc +++ src/3rdparty/chromium/third_party/blink/renderer/platform/audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc @@ -58,10 +58,10 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( const float sample_rate = SampleRate(); const float* delay_times = delay_times_.Data(); - int w_index = write_index_; const __m128 v_sample_rate = _mm_set1_ps(sample_rate); + const __m128 v_all_zeros = _mm_setzero_ps(); // The buffer length as a float and as an int so we don't need to constant // convert from one to the other. @@ -84,7 +84,10 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( int k = 0; for (int n = 0; n < number_of_loops; ++n, k += 4) { - const __m128 v_delay_time = _mm_loadu_ps(delay_times + k); + // It's possible that `delay_time` contains negative values. Make sure + // they are greater than zero. + const __m128 v_delay_time = _mm_max_ps(_mm_loadu_ps(delay_times + k), + v_all_zeros); const __m128 v_desired_delay_frames = _mm_mul_ps(v_delay_time, v_sample_rate); @@ -97,7 +100,8 @@ std::tuple AudioDelayDSPKernel::ProcessARateVector( WrapPositionVector(v_read_position, v_buffer_length_float); // Get indices into the buffer for the samples we need for interpolation. - const __m128i v_read_index1 = _mm_cvttps_epi32(v_read_position); + const __m128i v_read_index1 = WrapIndexVector( + _mm_cvttps_epi32(v_read_position), v_buffer_length_int); const __m128i v_read_index2 = WrapIndexVector( _mm_add_epi32(v_read_index1, _mm_set1_epi32(1)), v_buffer_length_int); From 4d4242d5d572e9427465e1833a711f50d4e30973 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Thu, 30 Nov 2023 13:53:00 -0500 Subject: [PATCH] [Backport] CVE-2024-0223: Heap buffer overflow in ANGLE (1/3) Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/angle/angle/+/5077407: Translator: Optimize field-name-collision check As each field of the struct was encountered, its name was linearly checked against previously added fields. That's O(n^2). The name collision check is now moved to when the struct is completely defined, and is done with an unordered_map. Bug: chromium:1505009 Change-Id: If28d738254a541450912eba4ed168424dad9d8be Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5077407 Commit-Queue: Shahbaz Youssefi Reviewed-by: Roman Lavrov Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530069 Reviewed-by: Michal Klocek --- .../src/compiler/translator/ParseContext.cpp | 39 +++++++++---------- .../src/compiler/translator/ParseContext.h | 5 +-- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp b/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp index b3d90a1a279..638cea22976 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp @@ -4665,6 +4665,9 @@ TIntermDeclaration *TParseContext::addInterfaceBlock( const TVector *arraySizes, const TSourceLoc &arraySizesLine) { + // Ensure there are no duplicate field names + checkDoesNotHaveDuplicateFieldNames(fieldList, nameLine); + const bool isGLPerVertex = blockName == "gl_PerVertex"; // gl_PerVertex is allowed to be redefined and therefore not reserved if (!isGLPerVertex) @@ -6172,28 +6175,25 @@ TDeclarator *TParseContext::parseStructArrayDeclarator(const ImmutableString &id return new TDeclarator(identifier, arraySizes, loc); } -void TParseContext::checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin, - const TFieldList::const_iterator end, - const ImmutableString &name, - const TSourceLoc &location) +void TParseContext::checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields, + const TSourceLoc &location) { - for (auto fieldIter = begin; fieldIter != end; ++fieldIter) + TUnorderedMap> + fieldNames; + for (TField *field : *fields) { - if ((*fieldIter)->name() == name) + // Note: operator[] adds this name to the map if it doesn't already exist, and initializes + // its value to 0. + uint32_t count = ++fieldNames[field->name()]; + if (count != 1) { - error(location, "duplicate field name in structure", name); + error(location, "Duplicate field name in structure", field->name()); } } } TFieldList *TParseContext::addStructFieldList(TFieldList *fields, const TSourceLoc &location) { - for (TFieldList::const_iterator fieldIter = fields->begin(); fieldIter != fields->end(); - ++fieldIter) - { - checkDoesNotHaveDuplicateFieldName(fields->begin(), fieldIter, (*fieldIter)->name(), - location); - } return fields; } @@ -6201,12 +6201,8 @@ TFieldList *TParseContext::combineStructFieldLists(TFieldList *processedFields, const TFieldList *newlyAddedFields, const TSourceLoc &location) { - for (TField *field : *newlyAddedFields) - { - checkDoesNotHaveDuplicateFieldName(processedFields->begin(), processedFields->end(), - field->name(), location); - processedFields->push_back(field); - } + processedFields->insert(processedFields->end(), newlyAddedFields->begin(), + newlyAddedFields->end()); return processedFields; } @@ -6299,7 +6295,10 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine, } } - // ensure we do not specify any storage qualifiers on the struct members + // Ensure there are no duplicate field names + checkDoesNotHaveDuplicateFieldNames(fieldList, structLine); + + // Ensure we do not specify any storage qualifiers on the struct members for (unsigned int typeListIndex = 0; typeListIndex < fieldList->size(); typeListIndex++) { TField &field = *(*fieldList)[typeListIndex]; diff --git a/chromium/third_party/angle/src/compiler/translator/ParseContext.h b/chromium/third_party/angle/src/compiler/translator/ParseContext.h index ee0cebe4f00..ca8dab269d6 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.h +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.h @@ -354,10 +354,7 @@ class TParseContext : angle::NonCopyable const TSourceLoc &loc, const TVector *arraySizes); - void checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin, - const TFieldList::const_iterator end, - const ImmutableString &name, - const TSourceLoc &location); + void checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields, const TSourceLoc &location); TFieldList *addStructFieldList(TFieldList *fields, const TSourceLoc &location); TFieldList *combineStructFieldLists(TFieldList *processedFields, const TFieldList *newlyAddedFields, From d0b3ab561418251a16c18ef5eba488294a209848 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Thu, 30 Nov 2023 14:12:42 -0500 Subject: [PATCH] [Backport] CVE-2024-0223: Heap buffer overflow in ANGLE (2/3) Cherry-pick of patch originally reviewed pn https://chromium-review.googlesource.com/c/angle/angle/+/5074629: Translator: Fail compilation if too many struct fields If there are too many struct fields, SPIR-V cannot be produced (as it has a hard limit of 16383 fields). The Nvidia GL driver has also been observed to fail when there are too many fields. Bug: chromium:1505009 Change-Id: If9b01716c1cab35a6e537da64421e29fe0eda91e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5074629 Commit-Queue: Shahbaz Youssefi Reviewed-by: Roman Lavrov Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530070 Reviewed-by: Michal Klocek --- .../src/compiler/translator/ParseContext.cpp | 19 +++++++++++++++++++ .../src/compiler/translator/ParseContext.h | 3 +++ 2 files changed, 22 insertions(+) diff --git a/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp b/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp index 638cea22976..5a4352b51ae 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp @@ -4665,6 +4665,8 @@ TIntermDeclaration *TParseContext::addInterfaceBlock( const TVector *arraySizes, const TSourceLoc &arraySizesLine) { + checkDoesNotHaveTooManyFields(blockName, fieldList, nameLine); + // Ensure there are no duplicate field names checkDoesNotHaveDuplicateFieldNames(fieldList, nameLine); @@ -6192,6 +6194,21 @@ void TParseContext::checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields } } +void TParseContext::checkDoesNotHaveTooManyFields(const ImmutableString &name, + const TFieldList *fields, + const TSourceLoc &location) +{ + // Check that there are not too many fields. SPIR-V has a limit of 16383 fields, and it would + // be reasonable to apply that limit to all outputs. For example, it was observed that 32768 + // fields cause the Nvidia GL driver to fail compilation, so such a limit is not too specific to + // SPIR-V. + constexpr size_t kMaxFieldCount = 16383; + if (fields->size() > kMaxFieldCount) + { + error(location, "Too many fields in the struct (limit is 16383)", name); + } +} + TFieldList *TParseContext::addStructFieldList(TFieldList *fields, const TSourceLoc &location) { return fields; @@ -6295,6 +6312,8 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine, } } + checkDoesNotHaveTooManyFields(structName, fieldList, structLine); + // Ensure there are no duplicate field names checkDoesNotHaveDuplicateFieldNames(fieldList, structLine); diff --git a/chromium/third_party/angle/src/compiler/translator/ParseContext.h b/chromium/third_party/angle/src/compiler/translator/ParseContext.h index ca8dab269d6..dca714a9e3d 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.h +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.h @@ -355,6 +355,9 @@ class TParseContext : angle::NonCopyable const TVector *arraySizes); void checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields, const TSourceLoc &location); + void checkDoesNotHaveTooManyFields(const ImmutableString &name, + const TFieldList *fields, + const TSourceLoc &location); TFieldList *addStructFieldList(TFieldList *fields, const TSourceLoc &location); TFieldList *combineStructFieldLists(TFieldList *processedFields, const TFieldList *newlyAddedFields, From fda3dd792d69ae6697fd63bebebd280a6c2aedd4 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Thu, 30 Nov 2023 15:42:32 -0500 Subject: [PATCH] [Backport] CVE-2024-0223: Heap buffer overflow in ANGLE (3/3) Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/angle/angle/+/5077408: Translator: Limit private variable size to 64KB This is indirectly fixing an issue where passing large arrays in SPIR-V such that an internal cast is needed (such as array inside interface block copied to local varaible) causes an overflow of the instruction length limit (in the absence of OpCopyLogical). By limiting the size of private variables to 32KB, this limitation is indirectly enforced. It was observed that all the test shaders added in this CL fail on the Nvidia OpenGL drivers, so such a limit seems to be reasonble. Bug: chromium:1505009 Change-Id: Ia36134b2bf8501a5b875814db3566be28b183e0f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5077408 Reviewed-by: Charlie Lao Reviewed-by: Geoff Lang Commit-Queue: Shahbaz Youssefi Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530091 Reviewed-by: Michal Klocek --- .../src/compiler/translator/Compiler.cpp | 12 +- .../ValidateTypeSizeLimitations.cpp | 131 +++++++++++++----- 2 files changed, 107 insertions(+), 36 deletions(-) diff --git a/chromium/third_party/angle/src/compiler/translator/Compiler.cpp b/chromium/third_party/angle/src/compiler/translator/Compiler.cpp index b5b4ccf1c2d..cc5d026099d 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/Compiler.cpp +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/Compiler.cpp @@ -770,11 +770,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, return false; } - if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) - { - return false; - } - if (!ValidateFragColorAndFragData(mShaderType, mShaderVersion, mSymbolTable, &mDiagnostics)) { return false; @@ -1053,6 +1048,13 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, return false; } + // Run after RemoveUnreferencedVariables, validate that the shader does not have excessively + // large variables. + if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) + { + return false; + } + // Built-in function emulation needs to happen after validateLimitations pass. GetGlobalPoolAllocator()->lock(); initBuiltInFunctionEmulator(&mBuiltInFunctionEmulator, compileOptions); diff --git a/chromium/third_party/angle/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/chromium/third_party/angle/src/compiler/translator/ValidateTypeSizeLimitations.cpp index f0ff9cb11ac..07e41d99354 100644 --- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ValidateTypeSizeLimitations.cpp +++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ValidateTypeSizeLimitations.cpp @@ -24,10 +24,11 @@ namespace // Arbitrarily enforce that all types declared with a size in bytes of over 2 GB will cause // compilation failure. // -// For local and global variables, the limit is much lower (16MB) as that much memory won't fit in +// For local and global variables, the limit is much lower (64KB) as that much memory won't fit in // the GPU registers anyway. -constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; -constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(16) * 1024 * 1024; +constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; +constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(64) * 1024; +constexpr size_t kMaxTotalPrivateVariableSizeInBytes = static_cast(16) * 1024 * 1024; // Traverses intermediate tree to ensure that the shader does not // exceed certain implementation-defined limits on the sizes of types. @@ -70,43 +71,111 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser continue; } - const TType &variableType = asSymbol->getType(); - - // Create a ShaderVariable from which to compute - // (conservative) sizing information. - ShaderVariable shaderVar; - setCommonVariableProperties(variableType, variable, &shaderVar); - - // Compute the std140 layout of this variable, assuming - // it's a member of a block (which it might not be). - Std140BlockEncoder layoutEncoder; - BlockEncoderVisitor visitor("", "", &layoutEncoder); - // Since the size limit's arbitrary, it doesn't matter - // whether the row-major layout is correctly determined. - bool isRowMajorLayout = false; - TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor); - if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes) + if (!validateVariableSize(variable, asSymbol->getLine())) { - error(asSymbol->getLine(), - "Size of declared variable exceeds implementation-defined limit", - asSymbol->getName()); return false; } + } + + return true; + } + + void visitFunctionPrototype(TIntermFunctionPrototype *node) override + { + const TFunction *function = node->getFunction(); + const size_t paramCount = function->getParamCount(); + + for (size_t paramIndex = 0; paramIndex < paramCount; ++paramIndex) + { + validateVariableSize(*function->getParam(paramIndex), node->getLine()); + } + } + + bool validateVariableSize(const TVariable &variable, const TSourceLoc &location) + { + const TType &variableType = variable.getType(); + + // Create a ShaderVariable from which to compute + // (conservative) sizing information. + ShaderVariable shaderVar; + setCommonVariableProperties(variableType, variable, &shaderVar); + + // Compute the std140 layout of this variable, assuming + // it's a member of a block (which it might not be). + Std140BlockEncoder layoutEncoder; + BlockEncoderVisitor visitor("", "", &layoutEncoder); + // Since the size limit's arbitrary, it doesn't matter + // whether the row-major layout is correctly determined. + bool isRowMajorLayout = false; + TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor); + if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes) + { + error(location, "Size of declared variable exceeds implementation-defined limit", + variable.name()); + return false; + } + + // Skip over struct declarations. As long as they are not used (or if they are used later + // in a less-restricted context (such as a UBO or SSBO)), they can be larger than + // kMaxPrivateVariableSizeInBytes. + if (variable.symbolType() == SymbolType::Empty && variableType.isStructSpecifier()) + { + return true; + } + + switch (variableType.getQualifier()) + { + // List of all types that need to be limited (for example because they cause overflows + // in drivers, or create trouble for the SPIR-V gen as the number of an instruction's + // arguments cannot be more than 64KB (see OutputSPIRVTraverser::cast)). + + // Local/global variables + case EvqTemporary: + case EvqGlobal: + case EvqConst: + + // Function arguments + case EvqParamIn: + case EvqParamOut: + case EvqParamInOut: + case EvqParamConst: + + // Varyings + case EvqVaryingIn: + case EvqVaryingOut: + case EvqSmoothOut: + case EvqFlatOut: + case EvqNoPerspectiveOut: + case EvqCentroidOut: + case EvqSampleOut: + case EvqSmoothIn: + case EvqFlatIn: + case EvqNoPerspectiveIn: + case EvqCentroidIn: + case EvqVertexOut: + case EvqFragmentIn: + case EvqGeometryIn: + case EvqGeometryOut: + case EvqPerVertexIn: + case EvqPerVertexOut: + case EvqPatchIn: + case EvqPatchOut: + case EvqTessControlIn: + case EvqTessControlOut: + case EvqTessEvaluationIn: + case EvqTessEvaluationOut: - const bool isPrivate = variableType.getQualifier() == EvqTemporary || - variableType.getQualifier() == EvqGlobal || - variableType.getQualifier() == EvqConst; - if (isPrivate) - { if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes) { - error(asSymbol->getLine(), + error(location, "Size of declared private variable exceeds implementation-defined limit", - asSymbol->getName()); + variable.name()); return false; } mTotalPrivateVariablesSize += layoutEncoder.getCurrentOffset(); - } + break; + default: + break; } return true; @@ -115,7 +184,7 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser void validateTotalPrivateVariableSize() { if (mTotalPrivateVariablesSize.ValueOrDefault(std::numeric_limits::max()) > - kMaxPrivateVariableSizeInBytes) + kMaxTotalPrivateVariableSizeInBytes) { mDiagnostics->error( TSourceLoc{}, From 5bbe9cf3b48b80901df6b446520581809f88e945 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Tue, 5 Dec 2023 13:36:53 -0500 Subject: [PATCH] [Backport] CVE-2024-0222: Use after free in ANGLE Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/angle/angle/+/5143829: M120: Vulkan: Don't crash when glCopyTexImage2D redefines itself The Vulkan backend marks a level being redefined as such before doing the copy. If a single-level texture was being redefined, it releases it so it can be immediately reallocated. If the source of the copy is the same texture, this causes a crash. This can be properly supported by using a temp image to do the copy, but that is not implemented in this change. Bug: chromium:1501798 Change-Id: I3a902b1e9eec41afd385d9c75a8c95dc986070a8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5143829 Reviewed-by: Cody Northrop Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530092 Reviewed-by: Michal Klocek --- .../libANGLE/renderer/vulkan/TextureVk.cpp | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/chromium/third_party/angle/src/libANGLE/renderer/vulkan/TextureVk.cpp b/chromium/third_party/angle/src/libANGLE/renderer/vulkan/TextureVk.cpp index 903def6e88e..fcd3bfa02f3 100644 --- src/3rdparty/chromium/third_party/angle/src/libANGLE/renderer/vulkan/TextureVk.cpp +++ src/3rdparty/chromium/third_party/angle/src/libANGLE/renderer/vulkan/TextureVk.cpp @@ -698,8 +698,28 @@ angle::Result TextureVk::copyImage(const gl::Context *context, gl::GetInternalFormatInfo(internalFormat, GL_UNSIGNED_BYTE); const vk::Format &vkFormat = renderer->getFormat(internalFormatInfo.sizedInternalFormat); + // The texture level being redefined might be the same as the one bound to the framebuffer. + // This _could_ be supported by using a temp image before redefining the level (and potentially + // discarding the image). However, this is currently unimplemented. + FramebufferVk *framebufferVk = vk::GetImpl(source); + RenderTargetVk *colorReadRT = framebufferVk->getColorReadRenderTarget(); + vk::ImageHelper *srcImage = &colorReadRT->getImageForCopy(); + const bool isCubeMap = index.getType() == gl::TextureType::CubeMap; + gl::LevelIndex levelIndex(getNativeImageIndex(index).getLevelIndex()); + const uint32_t layerIndex = index.hasLayer() ? index.getLayerIndex() : 0; + const uint32_t redefinedFace = isCubeMap ? layerIndex : 0; + const uint32_t sourceFace = isCubeMap ? colorReadRT->getLayerIndex() : 0; + const bool isSelfCopy = mImage == srcImage && levelIndex == colorReadRT->getLevelIndex() && + redefinedFace == sourceFace; + ANGLE_TRY(redefineLevel(context, index, vkFormat, newImageSize)); + if (isSelfCopy) + { + UNIMPLEMENTED(); + return angle::Result::Continue; + } + return copySubImageImpl(context, index, gl::Offset(0, 0, 0), sourceArea, internalFormatInfo, source); } @@ -1784,7 +1804,8 @@ angle::Result TextureVk::redefineLevel(const gl::Context *context, mImage->getLevelCount() == 1 && mImage->getFirstAllocatedLevel() == levelIndexGL; // If incompatible, and redefining the single-level image, release it so it can be - // recreated immediately. This is an optimization to avoid an extra copy. + // recreated immediately. This is needed so that the texture can be reallocated with + // the correct format/size. if (!isCompatibleRedefinition && isUpdateToSingleLevelImage) { releaseImage(contextVk); From 4ce1bbed853cba46f9ab6d1546e10253cc42f619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Br=C3=BCning?= Date: Sun, 14 Jan 2024 23:48:08 +0100 Subject: [PATCH] Fixup: [Backport] Security bug 1488199 Add register aliases following respective platform calling conventions. Change-Id: I8f844cd4db35393580f2a0adae6a4095584087a5 Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530630 Reviewed-by: Allan Sandfeld Jensen --- chromium/v8/src/codegen/arm/register-arm.h | 6 ++++++ chromium/v8/src/codegen/arm64/register-arm64.h | 6 ++++++ chromium/v8/src/codegen/loong64/register-loong64.h | 6 ++++++ chromium/v8/src/codegen/mips64/register-mips64.h | 6 ++++++ chromium/v8/src/codegen/ppc/register-ppc.h | 6 ++++++ chromium/v8/src/codegen/riscv/register-riscv.h | 5 +++++ chromium/v8/src/codegen/s390/register-s390.h | 6 ++++++ 7 files changed, 41 insertions(+) diff --git a/chromium/v8/src/codegen/arm/register-arm.h b/chromium/v8/src/codegen/arm/register-arm.h index 4edcddaa6f5a..40d07e4984e8 100644 --- src/3rdparty/chromium/v8/src/codegen/arm/register-arm.h +++ src/3rdparty/chromium/v8/src/codegen/arm/register-arm.h @@ -84,6 +84,12 @@ GENERAL_REGISTERS(DECLARE_REGISTER) #undef DECLARE_REGISTER constexpr Register no_reg = Register::no_reg(); +// ARM calling convention +constexpr Register arg_reg_1 = r0; +constexpr Register arg_reg_2 = r1; +constexpr Register arg_reg_3 = r2; +constexpr Register arg_reg_4 = r3; + // Returns the number of padding slots needed for stack pointer alignment. constexpr int ArgumentPaddingSlots(int argument_count) { // No argument padding required. diff --git a/chromium/v8/src/codegen/arm64/register-arm64.h b/chromium/v8/src/codegen/arm64/register-arm64.h index 24878e9d2567..a90412ae2061 100644 --- src/3rdparty/chromium/v8/src/codegen/arm64/register-arm64.h +++ src/3rdparty/chromium/v8/src/codegen/arm64/register-arm64.h @@ -525,6 +525,12 @@ ALIAS_REGISTER(VRegister, fp_scratch2, d31); #undef ALIAS_REGISTER +// Arm64 calling convention +constexpr Register arg_reg_1 = x0; +constexpr Register arg_reg_2 = x1; +constexpr Register arg_reg_3 = x2; +constexpr Register arg_reg_4 = x3; + // AreAliased returns true if any of the named registers overlap. Arguments set // to NoReg are ignored. The system stack pointer may be specified. V8_EXPORT_PRIVATE bool AreAliased( diff --git a/chromium/v8/src/codegen/loong64/register-loong64.h b/chromium/v8/src/codegen/loong64/register-loong64.h index 07c975223b26..724103587cf5 100644 --- src/3rdparty/chromium/v8/src/codegen/loong64/register-loong64.h +++ src/3rdparty/chromium/v8/src/codegen/loong64/register-loong64.h @@ -186,6 +186,12 @@ DEFINE_REGISTER_NAMES(Register, GENERAL_REGISTERS) DEFINE_REGISTER_NAMES(FPURegister, DOUBLE_REGISTERS) // Give alias names to registers for calling conventions. + +constexpr Register arg_reg_1 = a0; +constexpr Register arg_reg_2 = a1; +constexpr Register arg_reg_3 = a2; +constexpr Register arg_reg_4 = a3; + constexpr Register kReturnRegister0 = a0; constexpr Register kReturnRegister1 = a1; constexpr Register kReturnRegister2 = a2; diff --git a/chromium/v8/src/codegen/mips64/register-mips64.h b/chromium/v8/src/codegen/mips64/register-mips64.h index 00feb1c01c14..f3d5bd5c7985 100644 --- src/3rdparty/chromium/v8/src/codegen/mips64/register-mips64.h +++ src/3rdparty/chromium/v8/src/codegen/mips64/register-mips64.h @@ -278,6 +278,12 @@ DEFINE_REGISTER_NAMES(FPURegister, DOUBLE_REGISTERS) DEFINE_REGISTER_NAMES(MSARegister, SIMD128_REGISTERS) // Give alias names to registers for calling conventions. + +constexpr Register arg_reg_1 = a0; +constexpr Register arg_reg_2 = a1; +constexpr Register arg_reg_3 = a2; +constexpr Register arg_reg_4 = a3; + constexpr Register kReturnRegister0 = v0; constexpr Register kReturnRegister1 = v1; constexpr Register kReturnRegister2 = a0; diff --git a/chromium/v8/src/codegen/ppc/register-ppc.h b/chromium/v8/src/codegen/ppc/register-ppc.h index bdcb12b9d2d8..a2085e0e593b 100644 --- src/3rdparty/chromium/v8/src/codegen/ppc/register-ppc.h +++ src/3rdparty/chromium/v8/src/codegen/ppc/register-ppc.h @@ -152,6 +152,12 @@ constexpr Register kPtrComprCageBaseRegister = r27; // callee save constexpr Register kPtrComprCageBaseRegister = kRootRegister; #endif +// PPC64 calling convention +constexpr Register arg_reg_1 = r3; +constexpr Register arg_reg_2 = r4; +constexpr Register arg_reg_3 = r5; +constexpr Register arg_reg_4 = r6; + // Returns the number of padding slots needed for stack pointer alignment. constexpr int ArgumentPaddingSlots(int argument_count) { // No argument padding required. diff --git a/chromium/v8/src/codegen/riscv/register-riscv.h b/chromium/v8/src/codegen/riscv/register-riscv.h index c530c54b4ea1..d45fa80b5c91 100644 --- src/3rdparty/chromium/v8/src/codegen/riscv/register-riscv.h +++ src/3rdparty/chromium/v8/src/codegen/riscv/register-riscv.h @@ -271,6 +271,11 @@ DEFINE_REGISTER_NAMES(FPURegister, DOUBLE_REGISTERS) DEFINE_REGISTER_NAMES(VRegister, VECTOR_REGISTERS) // Give alias names to registers for calling conventions. +constexpr Register arg_reg_1 = a0; +constexpr Register arg_reg_2 = a1; +constexpr Register arg_reg_3 = a2; +constexpr Register arg_reg_4 = a3; + constexpr Register kReturnRegister0 = a0; constexpr Register kReturnRegister1 = a1; constexpr Register kReturnRegister2 = a2; diff --git a/chromium/v8/src/codegen/s390/register-s390.h b/chromium/v8/src/codegen/s390/register-s390.h index b3e5a49f2db5..6320135a2400 100644 --- src/3rdparty/chromium/v8/src/codegen/s390/register-s390.h +++ src/3rdparty/chromium/v8/src/codegen/s390/register-s390.h @@ -116,6 +116,12 @@ constexpr Register no_reg = Register::no_reg(); constexpr Register kRootRegister = r10; // Roots array pointer. constexpr Register cp = r13; // JavaScript context pointer. +// s390x calling convention +constexpr Register arg_reg_1 = r2; +constexpr Register arg_reg_2 = r3; +constexpr Register arg_reg_3 = r4; +constexpr Register arg_reg_4 = r5; + // Returns the number of padding slots needed for stack pointer alignment. constexpr int ArgumentPaddingSlots(int argument_count) { // No argument padding required. From d3328103b5e8336449108b8ba13549ced9caf404 Mon Sep 17 00:00:00 2001 From: Evan Stade Date: Fri, 15 Dec 2023 21:38:02 +0000 Subject: [PATCH] [Backport] Security bug 1511689 Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/deps/sqlite/+/5123910: Fix a spurious "misuse of aggregate function" error that could occur when an aggregate function was used within the FROM clause of a sub-select of the select that owns the aggregate. e.g. "SELECT (SELECT x FROM (SELECT sum(t1.a) AS x)) FROM t1". [forum:/forumpost/c9970a37ed | Forum post c9970a37ed]. FossilOrigin-Name: 4470f657d2069972d02a00983252dec1f814d90c0d8d0906e320e955111e8c11 (cherry picked from commit 5e4233a9e48b124d4d342b757b34e4ae849f5cf8) Bug: 1511689 Change-Id: I69263fc0a5fa66df5c09b964864568f2fc7a6ca5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/deps/sqlite/+/5123910 Auto-Submit: Evan Stade Commit-Queue: Ayu Ishii Reviewed-by: Ayu Ishii Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/530068 Reviewed-by: Allan Sandfeld Jensen --- chromium/third_party/sqlite/src/amalgamation/sqlite3.c | 6 +++++- chromium/third_party/sqlite/src/amalgamation_dev/sqlite3.c | 6 +++++- chromium/third_party/sqlite/src/src/resolve.c | 7 +++++-- chromium/third_party/sqlite/src/src/sqliteInt.h | 1 + 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/chromium/third_party/sqlite/src/amalgamation/sqlite3.c b/chromium/third_party/sqlite/src/amalgamation/sqlite3.c index d7766b7d7ec..b353aa88348 100644 --- src/3rdparty/chromium/third_party/sqlite/src/amalgamation/sqlite3.c +++ src/3rdparty/chromium/third_party/sqlite/src/amalgamation/sqlite3.c @@ -18804,6 +18804,7 @@ struct NameContext { int nRef; /* Number of names resolved by this context */ int nNcErr; /* Number of errors encountered while resolving names */ int ncFlags; /* Zero or more NC_* flags defined below */ + int nNestedSelect; /* Number of nested selects using this NC */ Select *pWinSelect; /* SELECT statement for any window functions */ }; @@ -104749,11 +104750,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){ while( pNC2 && sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0 ){ - pExpr->op2++; + pExpr->op2 += (1 + pNC2->nNestedSelect); pNC2 = pNC2->pNext; } assert( pDef!=0 || IN_RENAME_OBJECT ); if( pNC2 && pDef ){ + pExpr->op2 += pNC2->nNestedSelect; assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg ); assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg ); testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 ); @@ -105314,6 +105316,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ /* Recursively resolve names in all subqueries in the FROM clause */ + if( pOuterNC ) pOuterNC->nNestedSelect++; for(i=0; ipSrc->nSrc; i++){ SrcItem *pItem = &p->pSrc->a[i]; if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){ @@ -105338,6 +105341,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ } } } + if( pOuterNC ) pOuterNC->nNestedSelect--; /* Set up the local name-context to pass to sqlite3ResolveExprNames() to ** resolve the result-set expression list. diff --git a/chromium/third_party/sqlite/src/amalgamation_dev/sqlite3.c b/chromium/third_party/sqlite/src/amalgamation_dev/sqlite3.c index 0819ea6a615..5c72a44dd6b 100644 --- src/3rdparty/chromium/third_party/sqlite/src/amalgamation_dev/sqlite3.c +++ src/3rdparty/chromium/third_party/sqlite/src/amalgamation_dev/sqlite3.c @@ -18817,6 +18817,7 @@ struct NameContext { int nRef; /* Number of names resolved by this context */ int nNcErr; /* Number of errors encountered while resolving names */ int ncFlags; /* Zero or more NC_* flags defined below */ + int nNestedSelect; /* Number of nested selects using this NC */ Select *pWinSelect; /* SELECT statement for any window functions */ }; @@ -104762,11 +104763,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){ while( pNC2 && sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0 ){ - pExpr->op2++; + pExpr->op2 += (1 + pNC2->nNestedSelect); pNC2 = pNC2->pNext; } assert( pDef!=0 || IN_RENAME_OBJECT ); if( pNC2 && pDef ){ + pExpr->op2 += pNC2->nNestedSelect; assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg ); assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg ); testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 ); @@ -105327,6 +105329,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ /* Recursively resolve names in all subqueries in the FROM clause */ + if( pOuterNC ) pOuterNC->nNestedSelect++; for(i=0; ipSrc->nSrc; i++){ SrcItem *pItem = &p->pSrc->a[i]; if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){ @@ -105351,6 +105354,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ } } } + if( pOuterNC ) pOuterNC->nNestedSelect--; /* Set up the local name-context to pass to sqlite3ResolveExprNames() to ** resolve the result-set expression list. diff --git a/chromium/third_party/sqlite/src/src/resolve.c b/chromium/third_party/sqlite/src/src/resolve.c index 4b36ecca348..c5228a7f097 100644 --- src/3rdparty/chromium/third_party/sqlite/src/src/resolve.c +++ src/3rdparty/chromium/third_party/sqlite/src/src/resolve.c @@ -1211,11 +1211,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){ while( pNC2 && sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0 ){ - pExpr->op2++; + pExpr->op2 += (1 + pNC2->nNestedSelect); pNC2 = pNC2->pNext; } assert( pDef!=0 || IN_RENAME_OBJECT ); if( pNC2 && pDef ){ + pExpr->op2 += pNC2->nNestedSelect; assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg ); assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg ); testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 ); @@ -1776,6 +1777,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ /* Recursively resolve names in all subqueries in the FROM clause */ + if( pOuterNC ) pOuterNC->nNestedSelect++; for(i=0; ipSrc->nSrc; i++){ SrcItem *pItem = &p->pSrc->a[i]; if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){ @@ -1800,7 +1802,8 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ } } } - + if( pOuterNC ) pOuterNC->nNestedSelect--; + /* Set up the local name-context to pass to sqlite3ResolveExprNames() to ** resolve the result-set expression list. */ diff --git a/chromium/third_party/sqlite/src/src/sqliteInt.h b/chromium/third_party/sqlite/src/src/sqliteInt.h index 2614f4be458..07bc4def106 100644 --- src/3rdparty/chromium/third_party/sqlite/src/src/sqliteInt.h +++ src/3rdparty/chromium/third_party/sqlite/src/src/sqliteInt.h @@ -3321,6 +3321,7 @@ struct NameContext { int nRef; /* Number of names resolved by this context */ int nNcErr; /* Number of errors encountered while resolving names */ int ncFlags; /* Zero or more NC_* flags defined below */ + int nNestedSelect; /* Number of nested selects using this NC */ Select *pWinSelect; /* SELECT statement for any window functions */ }; From 54da597d9f7e7b9f331a15077eba6485b68280ab Mon Sep 17 00:00:00 2001 From: Toon Verwaest Date: Thu, 11 Jan 2024 10:47:17 +0100 Subject: [PATCH] [Backport] CVE-2024-0519: Out of bounds memory access in V8 Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/5192447: Merged: [runtime] Drop fast last-property deletion This interacts badly with other optimizations and isn't particularly common. Bug: chromium:1517354 (cherry picked from commit 389ea9be7d68bb189e16da79f6414edbd4f7594f) Change-Id: Ie16aa38e8984c4879491c0d9a0ca9df0e041fd1d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5192447 Auto-Submit: Toon Verwaest Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/branch-heads/12.0@{#32} Cr-Branched-From: ed7b4caf1fb8184ad9e24346c84424055d4d430a-refs/heads/12.0.267@{#1} Cr-Branched-From: 210e75b19db4352c9b78dce0bae11c2dc3077df4-refs/heads/main@{#90651} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/531577 Reviewed-by: Michal Klocek --- chromium/v8/src/runtime/runtime-object.cc | 174 ---------------------- 1 file changed, 174 deletions(-) diff --git a/chromium/v8/src/runtime/runtime-object.cc b/chromium/v8/src/runtime/runtime-object.cc index 2d4965549b2..ee8a9fbd147 100644 --- src/3rdparty/chromium/v8/src/runtime/runtime-object.cc +++ src/3rdparty/chromium/v8/src/runtime/runtime-object.cc @@ -81,184 +81,10 @@ MaybeHandle Runtime::HasProperty(Isolate* isolate, : ReadOnlyRoots(isolate).false_value_handle(); } -namespace { - -// This function sets the sentinel value in a deleted field. Thes sentinel has -// to look like a proper standalone object because the slack tracking may -// complete at any time. For this reason we use the filler map word. -// If V8_MAP_PACKING is enabled, then the filler map word is a packed filler -// map. Otherwise, the filler map word is the same as the filler map. -inline void ClearField(Isolate* isolate, JSObject object, FieldIndex index) { - if (index.is_inobject()) { - MapWord filler_map_word = - ReadOnlyRoots(isolate).one_pointer_filler_map_word(); -#ifndef V8_MAP_PACKING - DCHECK_EQ(filler_map_word.ToMap(), - ReadOnlyRoots(isolate).one_pointer_filler_map()); -#endif - int offset = index.offset(); - TaggedField::Release_Store(object, offset, filler_map_word); - } else { - object.property_array().set( - index.outobject_array_index(), - ReadOnlyRoots(isolate).one_pointer_filler_map()); - } -} - -void GeneralizeAllTransitionsToFieldAsMutable(Isolate* isolate, Handle map, - Handle name) { - InternalIndex descriptor(map->NumberOfOwnDescriptors()); - - Handle target_maps[kPropertyAttributesCombinationsCount]; - int target_maps_count = 0; - - // Collect all outgoing field transitions. - { - DisallowGarbageCollection no_gc; - TransitionsAccessor transitions(isolate, *map); - transitions.ForEachTransitionTo( - *name, - [&](Map target) { - DCHECK_EQ(descriptor, target.LastAdded()); - DCHECK_EQ(*name, target.GetLastDescriptorName(isolate)); - PropertyDetails details = target.GetLastDescriptorDetails(isolate); - // Currently, we track constness only for fields. - if (details.kind() == PropertyKind::kData && - details.constness() == PropertyConstness::kConst) { - target_maps[target_maps_count++] = handle(target, isolate); - } - DCHECK_IMPLIES(details.kind() == PropertyKind::kAccessor, - details.constness() == PropertyConstness::kConst); - }, - &no_gc); - CHECK_LE(target_maps_count, kPropertyAttributesCombinationsCount); - } - - for (int i = 0; i < target_maps_count; i++) { - Handle target = target_maps[i]; - PropertyDetails details = - target->instance_descriptors(isolate).GetDetails(descriptor); - Handle field_type( - target->instance_descriptors(isolate).GetFieldType(descriptor), - isolate); - MapUpdater::GeneralizeField(isolate, target, descriptor, - PropertyConstness::kMutable, - details.representation(), field_type); - DCHECK_EQ(PropertyConstness::kMutable, target->instance_descriptors(isolate) - .GetDetails(descriptor) - .constness()); - } -} - -bool DeleteObjectPropertyFast(Isolate* isolate, Handle receiver, - Handle raw_key) { - // This implements a special case for fast property deletion: when the - // last property in an object is deleted, then instead of normalizing - // the properties, we can undo the last map transition, with a few - // prerequisites: - // (1) The receiver must be a regular object and the key a unique name. - Handle receiver_map(receiver->map(), isolate); - if (receiver_map->IsSpecialReceiverMap()) return false; - DCHECK(receiver_map->IsJSObjectMap()); - - if (!raw_key->IsUniqueName()) return false; - Handle key = Handle::cast(raw_key); - // (2) The property to be deleted must be the last property. - int nof = receiver_map->NumberOfOwnDescriptors(); - if (nof == 0) return false; - InternalIndex descriptor(nof - 1); - Handle descriptors( - receiver_map->instance_descriptors(isolate), isolate); - if (descriptors->GetKey(descriptor) != *key) return false; - // (3) The property to be deleted must be deletable. - PropertyDetails details = descriptors->GetDetails(descriptor); - if (!details.IsConfigurable()) return false; - // (4) The map must have a back pointer. - Handle backpointer(receiver_map->GetBackPointer(), isolate); - if (!backpointer->IsMap()) return false; - Handle parent_map = Handle::cast(backpointer); - // (5) The last transition must have been caused by adding a property - // (and not any kind of special transition). - if (parent_map->NumberOfOwnDescriptors() != nof - 1) return false; - - // Preconditions successful. No more bailouts after this point. - - // Zap the property to avoid keeping objects alive. Zapping is not necessary - // for properties stored in the descriptor array. - if (details.location() == PropertyLocation::kField) { - DisallowGarbageCollection no_gc; - - // Invalidate slots manually later in case we delete an in-object tagged - // property. In this case we might later store an untagged value in the - // recorded slot. - isolate->heap()->NotifyObjectLayoutChange(*receiver, no_gc, - InvalidateRecordedSlots::kNo); - FieldIndex index = - FieldIndex::ForPropertyIndex(*receiver_map, details.field_index()); - // Special case deleting the last out-of object property. - if (!index.is_inobject() && index.outobject_array_index() == 0) { - DCHECK(!parent_map->HasOutOfObjectProperties()); - // Clear out the properties backing store. - receiver->SetProperties(ReadOnlyRoots(isolate).empty_fixed_array()); - } else { - ClearField(isolate, JSObject::cast(*receiver), index); - if (index.is_inobject()) { - // We need to clear the recorded slot in this case because in-object - // slack tracking might not be finished. This ensures that we don't - // have recorded slots in free space. - isolate->heap()->ClearRecordedSlot(*receiver, - receiver->RawField(index.offset())); - } - } - } - // If the {receiver_map} was marked stable before, then there could be - // optimized code that depends on the assumption that no object that - // reached this {receiver_map} transitions away from it without triggering - // the "deoptimize dependent code" mechanism. - receiver_map->NotifyLeafMapLayoutChange(isolate); - // Finally, perform the map rollback. - receiver->set_map(*parent_map, kReleaseStore); -#if VERIFY_HEAP - if (v8_flags.verify_heap) { - receiver->HeapObjectVerify(isolate); - receiver->property_array().PropertyArrayVerify(isolate); - } -#endif - - // If the {descriptor} was "const" so far, we need to update the - // {receiver_map} here, otherwise we could get the constants wrong, i.e. - // - // o.x = 1; - // [change o.x's attributes or reconfigure property kind] - // delete o.x; - // o.x = 2; - // - // could trick V8 into thinking that `o.x` is still 1 even after the second - // assignment. - - // Step 1: Migrate object to an up-to-date shape. - if (parent_map->is_deprecated()) { - JSObject::MigrateInstance(isolate, Handle::cast(receiver)); - parent_map = handle(receiver->map(), isolate); - } - - // Step 2: Mark outgoing transitions from the up-to-date version of the - // parent_map to same property name of any kind or attributes as mutable. - // Also migrate object to the up-to-date map to make the object shapes - // converge sooner. - GeneralizeAllTransitionsToFieldAsMutable(isolate, parent_map, key); - - return true; -} - -} // namespace - Maybe Runtime::DeleteObjectProperty(Isolate* isolate, Handle receiver, Handle key, LanguageMode language_mode) { - if (DeleteObjectPropertyFast(isolate, receiver, key)) return Just(true); - bool success = false; PropertyKey lookup_key(isolate, key, &success); if (!success) return Nothing(); From be7a2c69bf21ed5c1185840cc651608dfad95b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Inf=C3=BChr?= Date: Mon, 18 Dec 2023 09:15:00 +0100 Subject: [PATCH] [Backport] CVE-2024-0518: Type Confusion in V8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/5125960: [codegen] Install BytecodeArray last in SharedFunctionInfo Maglev assumes that when a SharedFunctionInfo has a BytecodeArray, then it should also have FeedbackMetadata. However, this may not hold with concurrent compilation when the SharedFunctionInfo is re-compiled after being flushed. Here the BytecodeArray was installed on the SFI before the FeedbackMetadata and a concurrent thread could observe the BytecodeArray but not the FeedbackMetadata. Drive-by: Reset the age field before setting the BytecodeArray as well. This ensures that the concurrent marker will not observe the old age for the new BytecodeArray. Bug: chromium:1507412 Change-Id: I8855ed7ecc50c4a47d2c89043d62ac053858bc75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5125960 Reviewed-by: Leszek Swirski Commit-Queue: Dominik Inführ Cr-Commit-Position: refs/heads/main@{#91568} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/531578 Reviewed-by: Michal Klocek --- chromium/v8/src/codegen/compiler.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chromium/v8/src/codegen/compiler.cc b/chromium/v8/src/codegen/compiler.cc index 5d3ee6e6a0c..47b58f1874f 100644 --- src/3rdparty/chromium/v8/src/codegen/compiler.cc +++ src/3rdparty/chromium/v8/src/codegen/compiler.cc @@ -686,11 +686,11 @@ void InstallUnoptimizedCode(UnoptimizedCompilationInfo* compilation_info, } #endif // V8_ENABLE_WEBASSEMBLY - shared_info->set_bytecode_array(*compilation_info->bytecode_array()); - Handle feedback_metadata = FeedbackMetadata::New( isolate, compilation_info->feedback_vector_spec()); shared_info->set_feedback_metadata(*feedback_metadata, kReleaseStore); + + shared_info->set_bytecode_array(*compilation_info->bytecode_array()); } else { #if V8_ENABLE_WEBASSEMBLY DCHECK(compilation_info->has_asm_wasm_data()); From 7b40abebdec3e2931c88010fedc96c49fbba1731 Mon Sep 17 00:00:00 2001 From: Mike Wasserman Date: Tue, 9 Jan 2024 01:07:39 +0000 Subject: [PATCH] [Backport] Security bug 1506535 Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5146875: [M120 merge] Speculative fix for UAF in content::WebContentsImpl::ExitFullscreenMode (cherry picked from commit c1cda70a433a0c625b280eb88ed6ff4f4feffa12) Bug: 1506535, 854815 Change-Id: Iace64d63f8cea2dbfbc761ad233db42451ec101c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5146875 Commit-Queue: John Abd-El-Malek Auto-Submit: Mike Wasserman Reviewed-by: John Abd-El-Malek Cr-Original-Commit-Position: refs/heads/main@{#1240353} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5178801 Cr-Commit-Position: refs/branch-heads/6099@{#1727} Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/531579 Reviewed-by: Michal Klocek --- chromium/content/browser/web_contents/web_contents_impl.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/chromium/content/browser/web_contents/web_contents_impl.cc b/chromium/content/browser/web_contents/web_contents_impl.cc index 59bbb727e6b..8b3f7055430 100644 --- src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc +++ src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc @@ -3600,7 +3600,12 @@ void WebContentsImpl::ExitFullscreenMode(bool will_cause_resize) { static_cast(view)->ExitFullscreenMode(); if (delegate_) { + // This may spin the message loop and destroy this object crbug.com/1506535 + base::WeakPtr weak_ptr = weak_factory_.GetWeakPtr(); delegate_->ExitFullscreenModeForTab(this); + if (!weak_ptr) { + return; + } if (keyboard_lock_widget_) delegate_->CancelKeyboardLockRequest(this); From 8ab0eb9f07be8cd735e03b5536fc2e361e70a5cf Mon Sep 17 00:00:00 2001 From: Lyra Rebane Date: Mon, 8 Jan 2024 13:39:46 +0000 Subject: [PATCH] [Backport] CVE-2024-0808: Integer underflow in WebUI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5177426: Verify resource order in data pack files This CL adds a resource order check when loading a data pack or calling DataPack::GetStringPiece to make sure the resources are ordered sequentially in memory. Bug: 1504936 Change-Id: Ie3bf1d9dbac937407355935a859a5daa9ce84350 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5059113 Commit-Queue: Peter Boström Cr-Commit-Position: refs/heads/main@{#1238675} (cherry picked from commit c4b2e6246ad0e95eaf0727bb25a2e4969155e989) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535516 Reviewed-by: Michal Klocek --- chromium/AUTHORS | 1 + chromium/ui/base/resource/data_pack.cc | 19 ++++++++++++++++++- .../ui/base/resource/data_pack_literal.cc | 12 ++++++++++++ chromium/ui/base/resource/data_pack_literal.h | 2 ++ .../ui/base/resource/data_pack_unittest.cc | 7 +++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/chromium/AUTHORS b/chromium/AUTHORS index ff6abe8d1135..772aab22c671 100644 --- src/3rdparty/chromium/AUTHORS +++ src/3rdparty/chromium/AUTHORS @@ -769,6 +769,7 @@ Luke Seunghoe Gu Luke Zarko Luoxi Pan Lu Yahan +Lyra Rebane Ma Aiguo Maarten Lankhorst Maciej Pawlowski diff --git a/chromium/ui/base/resource/data_pack.cc b/chromium/ui/base/resource/data_pack.cc index 74069c99d00a..6dc0985b78dd 100644 --- src/3rdparty/chromium/ui/base/resource/data_pack.cc +++ src/3rdparty/chromium/ui/base/resource/data_pack.cc @@ -310,7 +310,16 @@ bool DataPack::SanityCheckFileAndRegisterResources(size_t margin_to_skip, } } - // 3) Verify the aliases are within the appropriate bounds. + // 3) Verify the entries are ordered correctly. + for (size_t i = 0; i < resource_count_; ++i) { + if (resource_table_[i].file_offset > resource_table_[i + 1].file_offset) { + LOG(ERROR) << "Data pack file corruption: " + << "Entry #" << i + 1 << " before Entry #" << i << "."; + return false; + } + } + + // 4) Verify the aliases are within the appropriate bounds. for (size_t i = 0; i < alias_count_; ++i) { if (alias_table_[i].entry_index >= resource_count_) { LOG(ERROR) << "Data pack file corruption: " @@ -428,6 +437,14 @@ bool DataPack::GetStringPiece(uint16_t resource_id, << "file modified?"; return false; } + if (target->file_offset > next_entry->file_offset) { + size_t entry_index = target - resource_table_; + size_t next_index = next_entry - resource_table_; + LOG(ERROR) << "Entry #" << next_index << " in data pack is before Entry #" + << entry_index << ". This should have been caught when loading. " + << "Was the file modified?"; + return false; + } MaybePrintResourceId(resource_id); GetStringPieceFromOffset(target->file_offset, next_entry->file_offset, diff --git a/chromium/ui/base/resource/data_pack_literal.cc b/chromium/ui/base/resource/data_pack_literal.cc index caac0709b42b..4197ea03fd68 100644 --- src/3rdparty/chromium/ui/base/resource/data_pack_literal.cc +++ src/3rdparty/chromium/ui/base/resource/data_pack_literal.cc @@ -89,6 +89,18 @@ const uint8_t kSampleCorruptPakContents[] = { const size_t kSampleCorruptPakSize = sizeof(kSampleCorruptPakContents); +const uint8_t kSampleMisorderedPakContents[] = { + 0x05, 0x00, 0x00, 0x00, // version + 0x01, 0x00, 0x00, 0x00, // encoding + padding + 0x02, 0x00, 0x00, 0x00, // num_resources, num_aliases + 0x06, 0x00, 0x2a, 0x00, 0x00, 0x00, // index entry 6 (wrong order) + 0x04, 0x00, 0x1e, 0x00, 0x00, 0x00, // index entry 4 + 0x00, 0x00, 0x36, 0x00, 0x00, 0x00, // extra entry for the size of last + 't', 'h', 'i', 's', ' ', 'i', 's', ' ', 'i', 'd', ' ', '4', + 't', 'h', 'i', 's', ' ', 'i', 's', ' ', 'i', 'd', ' ', '6'}; + +const size_t kSampleMisorderedPakSize = sizeof(kSampleMisorderedPakContents); + const uint8_t kSamplePakContents2x[] = { 0x04, 0x00, 0x00, 0x00, // header(version 0x01, 0x00, 0x00, 0x00, // no. entries diff --git a/chromium/ui/base/resource/data_pack_literal.h b/chromium/ui/base/resource/data_pack_literal.h index eb5a94895f2d..9173ce149935 100644 --- src/3rdparty/chromium/ui/base/resource/data_pack_literal.h +++ src/3rdparty/chromium/ui/base/resource/data_pack_literal.h @@ -22,6 +22,8 @@ extern const uint8_t kEmptyPakContents[]; extern const size_t kEmptyPakSize; extern const uint8_t kSampleCorruptPakContents[]; extern const size_t kSampleCorruptPakSize; +extern const uint8_t kSampleMisorderedPakContents[]; +extern const size_t kSampleMisorderedPakSize; } // namespace ui diff --git a/chromium/ui/base/resource/data_pack_unittest.cc b/chromium/ui/base/resource/data_pack_unittest.cc index 25b33b813ac4..0a4a169ca225 100644 --- src/3rdparty/chromium/ui/base/resource/data_pack_unittest.cc +++ src/3rdparty/chromium/ui/base/resource/data_pack_unittest.cc @@ -366,4 +366,11 @@ TEST(DataPackTest, ModifiedWhileUsed) { } #endif +TEST(DataPackTest, Misordered) { + DataPack pack(k100Percent); + + ASSERT_FALSE(pack.LoadFromBuffer( + {kSampleMisorderedPakContents, kSampleMisorderedPakSize})); +} + } // namespace ui From 46069ff72f6e1d6fe75bd2c04350bcd74b308923 Mon Sep 17 00:00:00 2001 From: Hongchan Choi Date: Fri, 12 Jan 2024 22:57:22 +0000 Subject: [PATCH] [Backport] CVE-2024-0807: Use after free in WebAudio Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5225523: Update rendering state of automatic pull nodes before graph rendering M114 merge issues: third_party/blink/renderer/modules/webaudio/analyser_handler.cc: PullInputs/CheckNumberOfChannelsForInput not present in 114. In rare cases, the rendering fan out count of automatic pull node does not match the main thread fan out count after recreating a platform destination followed by disconnection. This CL forces the update of the rendering state of automatic pull nodes before graph rendering to make sure that fan out counts are synchronized before executing the audio processing function call. NOTE: This change makes 2 WPTs fail. The follow-up work is planned to address them once this patch is merged. Bug: 1505080 Test: Locally confirmed that ASAN doesn't crash on all repro cases. Change-Id: I6768cd8bc64525ea9d56a19b9c58439e9cdab9a8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5131958 Commit-Queue: Hongchan Choi Cr-Commit-Position: refs/heads/main@{#1246718} (cherry picked from commit f4bffa09b46c21147431179e1e6dd2b27bc35fbc) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535517 Reviewed-by: Michal Klocek --- .../renderer/modules/webaudio/analyser_handler.cc | 14 ++++++++++++-- .../modules/webaudio/audio_worklet_handler.cc | 7 +++++-- .../modules/webaudio/audio_worklet_processor.cc | 6 ++++++ .../modules/webaudio/deferred_task_handler.cc | 10 ++++++++++ 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/chromium/third_party/blink/renderer/modules/webaudio/analyser_handler.cc b/chromium/third_party/blink/renderer/modules/webaudio/analyser_handler.cc index c823c923a1cc..87a1f109a28c 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/analyser_handler.cc +++ src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/analyser_handler.cc @@ -39,9 +39,14 @@ AnalyserHandler::~AnalyserHandler() { } void AnalyserHandler::Process(uint32_t frames_to_process) { - AudioBus* output_bus = Output(0).Bus(); + DCHECK(Context()->IsAudioThread()); - if (!IsInitialized()) { + // It's possible that output is not connected. Assign nullptr to indicate + // such case. + AudioBus* output_bus = + Output(0).RenderingFanOutCount() > 0 ? Output(0).Bus() : nullptr; + + if (!IsInitialized() && output_bus) { output_bus->Zero(); return; } @@ -53,6 +58,11 @@ void AnalyserHandler::Process(uint32_t frames_to_process) { // Analyser reflects the current input. analyser_.WriteInput(input_bus.get(), frames_to_process); + // Subsequent steps require `output_bus` to be valid. + if (!output_bus) { + return; + } + if (!Input(0).IsConnected()) { // No inputs, so clear the output, and propagate the silence hint. output_bus->Zero(); diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_handler.cc b/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_handler.cc index 7f591531ad6f..b2b1500d3aab 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_handler.cc +++ src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_handler.cc @@ -114,12 +114,15 @@ void AudioWorkletHandler::Process(uint32_t frames_to_process) { // We also need to check if the global scope is valid before we request // the rendering in the AudioWorkletGlobalScope. if (processor_ && !processor_->hasErrorOccurred()) { - // If the input is not connected, inform the processor with nullptr. + // If the input or the output is not connected, inform the processor with + // nullptr. for (unsigned i = 0; i < NumberOfInputs(); ++i) { inputs_[i] = Input(i).IsConnected() ? Input(i).Bus() : nullptr; } for (unsigned i = 0; i < NumberOfOutputs(); ++i) { - outputs_[i] = WrapRefCounted(Output(i).Bus()); + outputs_[i] = Output(i).RenderingFanOutCount() > 0 + ? WrapRefCounted(Output(i).Bus()) + : nullptr; } for (const auto& param_name : param_value_map_.Keys()) { diff --git a/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc b/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc index 1f884cb12b43..c47e39effa40 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc +++ src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/audio_worklet_processor.cc @@ -367,6 +367,12 @@ void AudioWorkletProcessor::CopyArrayBuffersToPort( for (uint32_t bus_index = 0; bus_index < audio_port.size(); ++bus_index) { const scoped_refptr& audio_bus = audio_port[bus_index]; + + // nullptr indicates the output bus is not connected. Do not proceed. + if (!audio_bus) { + break; + } + for (uint32_t channel_index = 0; channel_index < audio_bus->NumberOfChannels(); ++channel_index) { auto backing_store = array_buffers[bus_index][channel_index] diff --git a/chromium/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc b/chromium/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc index fa1de8f37b9b..4730383dafa9 100644 --- src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc +++ src/3rdparty/chromium/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc @@ -172,6 +172,16 @@ void DeferredTaskHandler::UpdateAutomaticPullNodes() { base::AutoTryLock try_locker(automatic_pull_handlers_lock_); if (try_locker.is_acquired()) { rendering_automatic_pull_handlers_.assign(automatic_pull_handlers_); + + // In rare cases, it is possible for automatic pull nodes' output bus + // to become stale. Make sure update their rendering output counts. + // crbug.com/1505080. + for (auto& handler : rendering_automatic_pull_handlers_) { + for (unsigned i = 0; i < handler->NumberOfOutputs(); ++i) { + handler->Output(i).UpdateRenderingState(); + } + } + automatic_pull_handlers_need_updating_ = false; } } From 0801943eea5309d1912bac96ed15af49b9f4e532 Mon Sep 17 00:00:00 2001 From: Cheng Chen Date: Thu, 7 Dec 2023 12:17:23 -0800 Subject: [PATCH] [Backport] Security bug 1511389 (1/2) Manual partial cherry-pick of patch originally reviewed on https://aomedia-review.googlesource.com/c/aom/+/184763: Do not use adaptive error estimate When the reference frame size is different than the current, we will not use adaptive error estimate. STATS_CHANGED Bug: b:314858909 Change-Id: Ic64d9b4a1d94889d7283c044b17ffc24627478d7 Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535518 Reviewed-by: Michal Klocek --- .../libaom/source/libaom/av1/encoder/ratectrl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/ratectrl.c b/chromium/third_party/libaom/source/libaom/av1/encoder/ratectrl.c index 4ea1c9a3e33..c7b503d80a2 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ratectrl.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ratectrl.c @@ -187,8 +187,7 @@ int av1_rc_bits_per_mb(const AV1_COMP *cpi, FRAME_TYPE frame_type, int qindex, assert(correction_factor <= MAX_BPB_FACTOR && correction_factor >= MIN_BPB_FACTOR); - if (frame_type != KEY_FRAME && accurate_estimate) { - assert(cpi->rec_sse != UINT64_MAX); + if (frame_type != KEY_FRAME && accurate_estimate && cpi->rec_sse != UINT64_MAX) { const int mbs = cm->mi_params.MBs; const double sse_sqrt = (double)((int)sqrt((double)(cpi->rec_sse)) << BPER_MB_NORMBITS) / @@ -2021,6 +2020,13 @@ static void rc_compute_variance_onepass_rt(AV1_COMP *cpi) { // TODO(yunqing): support scaled reference frames. if (cpi->scaled_ref_buf[LAST_FRAME - 1]) return; + for (int i = 0; i < 2; ++i) { + if (unscaled_src->widths[i] != yv12->widths[i] || + unscaled_src->heights[i] != yv12->heights[i]) { + return; + } + } + const int num_mi_cols = cm->mi_params.mi_cols; const int num_mi_rows = cm->mi_params.mi_rows; const BLOCK_SIZE bsize = BLOCK_64X64; From 1a76ec5bc55594a7feada7c510949450d489996b Mon Sep 17 00:00:00 2001 From: Remya Prakasan Date: Mon, 8 May 2023 15:03:27 +0530 Subject: [PATCH] [Backport] Dependency for security bug 1511389 (1/1) Manual cherry-pick of patch originally reviewed on https://aomedia-review.googlesource.com/c/aom/+/175041: Add support for dynamic allocation of thread data Added support for reallocation of thread data when the workers for multi-threading in encode stage changes with frame resizing. Also modified TestExternalResizeWorks of ResizeRealtimeTest to test this scenario. BUG=aomedia:3429 Change-Id: Ieee94b229274e942203c9fc7dffd59a9a3fb5c26 Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535519 Reviewed-by: Michal Klocek --- .../libaom/source/libaom/av1/av1_cx_iface.c | 14 ++++++++ .../source/libaom/av1/encoder/encoder.c | 34 ------------------- .../source/libaom/av1/encoder/encoder.h | 5 +++ .../source/libaom/av1/encoder/encoder_alloc.h | 34 +++++++++++++++++++ .../source/libaom/av1/encoder/ethread.c | 5 +++ 5 files changed, 58 insertions(+), 34 deletions(-) diff --git a/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c b/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c index 3e764dd6ca6..1d114779c83 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c @@ -25,6 +25,7 @@ #include "av1/av1_iface_common.h" #include "av1/encoder/bitstream.h" #include "av1/encoder/encoder.h" +#include "av1/encoder/encoder_alloc.h" #include "av1/encoder/encoder_utils.h" #include "av1/encoder/ethread.h" #include "av1/encoder/external_partition.h" @@ -3095,6 +3096,19 @@ static aom_codec_err_t encoder_encode(aom_codec_alg_priv_t *ctx, } #endif // CONFIG_MULTITHREAD } + + // Re-allocate thread data if workers for encoder multi-threading stage + // exceeds prev_num_enc_workers. + const int num_enc_workers = + av1_get_num_mod_workers_for_alloc(&ppi->p_mt_info, MOD_ENC); + if (ppi->p_mt_info.prev_num_enc_workers < num_enc_workers && + num_enc_workers <= ppi->p_mt_info.num_workers) { + free_thread_data(ppi); + for (int j = 0; j < ppi->num_fp_contexts; j++) + aom_free(ppi->parallel_cpi[j]->td.tctx); + av1_init_tile_thread_data(ppi, cpi->oxcf.pass == AOM_RC_FIRST_PASS); + } + for (int i = 0; i < ppi->num_fp_contexts; i++) { av1_init_frame_mt(ppi, ppi->parallel_cpi[i]); } diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c b/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c index 72cb92bbb22..c2bf5b9b344 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c @@ -1569,40 +1569,6 @@ static AOM_INLINE void terminate_worker_data(AV1_PRIMARY *ppi) { } } -// Deallocate allocated thread_data. -static AOM_INLINE void free_thread_data(AV1_PRIMARY *ppi) { - PrimaryMultiThreadInfo *const p_mt_info = &ppi->p_mt_info; - for (int t = 1; t < p_mt_info->num_workers; ++t) { - EncWorkerData *const thread_data = &p_mt_info->tile_thr_data[t]; - thread_data->td = thread_data->original_td; - aom_free(thread_data->td->tctx); - aom_free(thread_data->td->palette_buffer); - aom_free(thread_data->td->tmp_conv_dst); - release_compound_type_rd_buffers(&thread_data->td->comp_rd_buffer); - for (int j = 0; j < 2; ++j) { - aom_free(thread_data->td->tmp_pred_bufs[j]); - } - aom_free(thread_data->td->pixel_gradient_info); - aom_free(thread_data->td->src_var_info_of_4x4_sub_blocks); - release_obmc_buffers(&thread_data->td->obmc_buffer); - aom_free(thread_data->td->vt64x64); - - for (int x = 0; x < 2; x++) { - for (int y = 0; y < 2; y++) { - aom_free(thread_data->td->hash_value_buffer[x][y]); - thread_data->td->hash_value_buffer[x][y] = NULL; - } - } - aom_free(thread_data->td->counts); - av1_free_pmc(thread_data->td->firstpass_ctx, - ppi->seq_params.monochrome ? 1 : MAX_MB_PLANE); - thread_data->td->firstpass_ctx = NULL; - av1_free_shared_coeff_buffer(&thread_data->td->shared_coeff_buf); - av1_free_sms_tree(thread_data->td); - aom_free(thread_data->td); - } -} - void av1_remove_primary_compressor(AV1_PRIMARY *ppi) { if (!ppi) return; #if !CONFIG_REALTIME_ONLY diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.h b/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.h index a95ea2505d7..153b3665f23 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.h +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.h @@ -1631,6 +1631,11 @@ typedef struct PrimaryMultiThreadInfo { * Number of primary workers created for multi-threading. */ int p_num_workers; + + /*! + * Tracks the number of workers in encode stage multi-threading. + */ + int prev_num_enc_workers; } PrimaryMultiThreadInfo; /*! diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/encoder_alloc.h b/chromium/third_party/libaom/source/libaom/av1/encoder/encoder_alloc.h index a4aef85aedb..27b5546371a 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder_alloc.h +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder_alloc.h @@ -398,6 +398,40 @@ static AOM_INLINE YV12_BUFFER_CONFIG *realloc_and_scale_source( return &cpi->scaled_source; } +// Deallocate allocated thread_data. +static AOM_INLINE void free_thread_data(AV1_PRIMARY *ppi) { + PrimaryMultiThreadInfo *const p_mt_info = &ppi->p_mt_info; + for (int t = 1; t < p_mt_info->num_workers; ++t) { + EncWorkerData *const thread_data = &p_mt_info->tile_thr_data[t]; + thread_data->td = thread_data->original_td; + aom_free(thread_data->td->tctx); + aom_free(thread_data->td->palette_buffer); + aom_free(thread_data->td->tmp_conv_dst); + release_compound_type_rd_buffers(&thread_data->td->comp_rd_buffer); + for (int j = 0; j < 2; ++j) { + aom_free(thread_data->td->tmp_pred_bufs[j]); + } + aom_free(thread_data->td->pixel_gradient_info); + aom_free(thread_data->td->src_var_info_of_4x4_sub_blocks); + release_obmc_buffers(&thread_data->td->obmc_buffer); + aom_free(thread_data->td->vt64x64); + + for (int x = 0; x < 2; x++) { + for (int y = 0; y < 2; y++) { + aom_free(thread_data->td->hash_value_buffer[x][y]); + thread_data->td->hash_value_buffer[x][y] = NULL; + } + } + aom_free(thread_data->td->counts); + av1_free_pmc(thread_data->td->firstpass_ctx, + ppi->seq_params.monochrome ? 1 : MAX_MB_PLANE); + thread_data->td->firstpass_ctx = NULL; + av1_free_shared_coeff_buffer(&thread_data->td->shared_coeff_buf); + av1_free_sms_tree(thread_data->td); + aom_free(thread_data->td); + } +} + #ifdef __cplusplus } // extern "C" #endif diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c b/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c index 1c8631ae1fd..8c62b2107c3 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c @@ -777,6 +777,7 @@ void av1_init_tile_thread_data(AV1_PRIMARY *ppi, int is_first_pass) { int num_workers = p_mt_info->num_workers; int num_enc_workers = av1_get_num_mod_workers_for_alloc(p_mt_info, MOD_ENC); + assert(num_enc_workers <= num_workers); for (int i = num_workers - 1; i >= 0; i--) { EncWorkerData *const thread_data = &p_mt_info->tile_thr_data[i]; @@ -886,6 +887,10 @@ void av1_init_tile_thread_data(AV1_PRIMARY *ppi, int is_first_pass) { } } } + + // Record the number of workers in encode stage multi-threading for which + // allocation is done. + p_mt_info->prev_num_enc_workers = num_enc_workers; } void av1_create_workers(AV1_PRIMARY *ppi, int num_workers) { From 9e80e8bff6bd41a61b589ecb6b006c1711e83431 Mon Sep 17 00:00:00 2001 From: Cheng Chen Date: Tue, 5 Dec 2023 16:34:43 -0800 Subject: [PATCH] [Backport] Security bug 1511389 (2/2) Manual cherry-pick of patch originally reviewed on https://aomedia-review.googlesource.com/c/aom/+/184761: Recreate workers if necessary As shown in the unit test, if the number of workers increases, we need to propoerly recreate new workers. Bug: b:310455204 Change-Id: I0fafb11c10ffba209a4c49f4a531cfbf09c9c2b4 Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535520 Reviewed-by: Michal Klocek --- .../libaom/source/libaom/av1/av1_cx_iface.c | 15 ++++++++++++++- .../libaom/source/libaom/av1/encoder/encoder.c | 16 ++++------------ .../libaom/source/libaom/av1/encoder/ethread.c | 12 ++++++++++++ .../libaom/source/libaom/av1/encoder/ethread.h | 2 ++ 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c b/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c index 1d114779c83..618021a768d 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/av1_cx_iface.c @@ -3078,12 +3078,25 @@ static aom_codec_err_t encoder_encode(aom_codec_alg_priv_t *ctx, av1_compute_num_workers_for_mt(cpi); num_workers = av1_get_max_num_workers(cpi); } - if ((num_workers > 1) && (ppi->p_mt_info.num_workers == 0)) { + if (num_workers > 1 && ppi->p_mt_info.num_workers < num_workers) { // Obtain the maximum no. of frames that can be supported in a parallel // encode set. if (is_stat_consumption_stage(cpi)) { ppi->num_fp_contexts = av1_compute_num_fp_contexts(ppi, &cpi->oxcf); } + if (ppi->p_mt_info.num_workers > 0) { + av1_terminate_workers(ppi); + free_thread_data(ppi); + aom_free(ppi->p_mt_info.tile_thr_data); + ppi->p_mt_info.tile_thr_data = NULL; + aom_free(ppi->p_mt_info.workers); + ppi->p_mt_info.workers = NULL; + ppi->p_mt_info.num_workers = 0; + for (int j = 0; j < ppi->num_fp_contexts; j++) { + aom_free(ppi->parallel_cpi[j]->td.tctx); + ppi->parallel_cpi[j]->td.tctx = NULL; + } + } av1_create_workers(ppi, num_workers); av1_init_tile_thread_data(ppi, cpi->oxcf.pass == AOM_RC_FIRST_PASS); #if CONFIG_MULTITHREAD diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c b/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c index c2bf5b9b344..5825ee00f76 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/encoder.c @@ -1558,17 +1558,6 @@ AV1_COMP *av1_create_compressor(AV1_PRIMARY *ppi, const AV1EncoderConfig *oxcf, snprintf((H) + strlen(H), sizeof(H) - strlen(H), (T), (V)) #endif // CONFIG_INTERNAL_STATS -// This function will change the state and free the mutex of corresponding -// workers and terminate the object. The object can not be re-used unless a call -// to reset() is made. -static AOM_INLINE void terminate_worker_data(AV1_PRIMARY *ppi) { - PrimaryMultiThreadInfo *const p_mt_info = &ppi->p_mt_info; - for (int t = p_mt_info->num_workers - 1; t >= 0; --t) { - AVxWorker *const worker = &p_mt_info->workers[t]; - aom_get_worker_interface()->end(worker); - } -} - void av1_remove_primary_compressor(AV1_PRIMARY *ppi) { if (!ppi) return; #if !CONFIG_REALTIME_ONLY @@ -1596,11 +1585,14 @@ void av1_remove_primary_compressor(AV1_PRIMARY *ppi) { av1_tpl_dealloc(&tpl_data->tpl_mt_sync); #endif - terminate_worker_data(ppi); + av1_terminate_workers(ppi); free_thread_data(ppi); aom_free(ppi->p_mt_info.tile_thr_data); + ppi->p_mt_info.tile_thr_data = NULL; aom_free(ppi->p_mt_info.workers); + ppi->p_mt_info.workers = NULL; + ppi->p_mt_info.num_workers = 0; aom_free(ppi); } diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c b/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c index 8c62b2107c3..d59c4f1d57e 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.c @@ -896,6 +896,7 @@ void av1_init_tile_thread_data(AV1_PRIMARY *ppi, int is_first_pass) { void av1_create_workers(AV1_PRIMARY *ppi, int num_workers) { PrimaryMultiThreadInfo *const p_mt_info = &ppi->p_mt_info; const AVxWorkerInterface *const winterface = aom_get_worker_interface(); + assert(p_mt_info->num_workers == 0); AOM_CHECK_MEM_ERROR(&ppi->error, p_mt_info->workers, aom_malloc(num_workers * sizeof(*p_mt_info->workers))); @@ -927,6 +928,17 @@ void av1_create_workers(AV1_PRIMARY *ppi, int num_workers) { } } +// This function will change the state and free the mutex of corresponding +// workers and terminate the object. The object can not be re-used unless a call +// to reset() is made. +void av1_terminate_workers(AV1_PRIMARY *ppi) { + PrimaryMultiThreadInfo *const p_mt_info = &ppi->p_mt_info; + for (int t = 0; t < p_mt_info->num_workers; ++t) { + AVxWorker *const worker = &p_mt_info->workers[t]; + aom_get_worker_interface()->end(worker); + } +} + // This function returns 1 if frame parallel encode is supported for // the current configuration. Returns 0 otherwise. static AOM_INLINE int is_fpmt_config(AV1_PRIMARY *ppi, AV1EncoderConfig *oxcf) { diff --git a/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.h b/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.h index 6c4bce4db57..942ed64510b 100644 --- src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.h +++ src/3rdparty/chromium/third_party/libaom/source/libaom/av1/encoder/ethread.h @@ -87,6 +87,8 @@ int av1_get_max_num_workers(const AV1_COMP *cpi); void av1_create_workers(AV1_PRIMARY *ppi, int num_workers); +void av1_terminate_workers(AV1_PRIMARY *ppi); + void av1_init_frame_mt(AV1_PRIMARY *ppi, AV1_COMP *cpi); void av1_init_cdef_worker(AV1_COMP *cpi); From da29c7f0b3e2044a7e597498a6fb62a306661f03 Mon Sep 17 00:00:00 2001 From: Andrey Kosyakov Date: Fri, 17 Nov 2023 17:48:22 +0000 Subject: [PATCH] [Backport] CVE-2024-0810: Insufficient policy enforcement in DevTools Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5039174: Do not let chrome.debugger extensions invoke Network.getAllCookies Network.getAllCookies is deprecated in favor of Storage.getCookies and the latter is not allowed for extensions, so we shouldn't let extensions use the former either. Bug: 1496250 Change-Id: I3e97e9249dbba61d1f7951ed22ef9b1bef9f2355 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039174 Reviewed-by: Danil Somsikov Commit-Queue: Andrey Kosyakov Cr-Commit-Position: refs/heads/main@{#1226203} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535521 Reviewed-by: Michal Klocek --- .../browser/devtools/protocol/network_handler.cc | 14 ++++++++++---- .../browser/devtools/protocol/network_handler.h | 6 ++++-- .../devtools/render_frame_devtools_agent_host.cc | 3 ++- .../devtools/service_worker_devtools_agent_host.cc | 3 ++- .../devtools/shared_worker_devtools_agent_host.cc | 3 ++- .../browser/devtools/worker_devtools_agent_host.cc | 3 ++- 6 files changed, 22 insertions(+), 10 deletions(-) diff --git a/chromium/content/browser/devtools/protocol/network_handler.cc b/chromium/content/browser/devtools/protocol/network_handler.cc index cfab47157112..7de14e0e4b95 100644 --- src/3rdparty/chromium/content/browser/devtools/protocol/network_handler.cc +++ src/3rdparty/chromium/content/browser/devtools/protocol/network_handler.cc @@ -109,7 +109,8 @@ using DeleteCookiesCallback = Network::Backend::DeleteCookiesCallback; using ClearBrowserCookiesCallback = Network::Backend::ClearBrowserCookiesCallback; -const char kInvalidCookieFields[] = "Invalid cookie fields"; +static constexpr char kInvalidCookieFields[] = "Invalid cookie fields"; +static constexpr char kNotAllowedError[] = "Not allowed"; Network::CertificateTransparencyCompliance SerializeCTPolicyCompliance( net::ct::CTPolicyCompliance ct_compliance) { @@ -1027,11 +1028,14 @@ NetworkHandler::NetworkHandler( const base::UnguessableToken& devtools_token, DevToolsIOContext* io_context, base::RepeatingClosure update_loader_factories_callback, - bool allow_file_access) + bool allow_file_access, + bool client_is_trusted) : DevToolsDomainHandler(Network::Metainfo::domainName), host_id_(host_id), devtools_token_(devtools_token), io_context_(io_context), + allow_file_access_(allow_file_access), + client_is_trusted_(client_is_trusted), browser_context_(nullptr), storage_partition_(nullptr), host_(nullptr), @@ -1042,8 +1046,7 @@ NetworkHandler::NetworkHandler( bypass_service_worker_(false), cache_disabled_(false), update_loader_factories_callback_( - std::move(update_loader_factories_callback)), - allow_file_access_(allow_file_access) { + std::move(update_loader_factories_callback)) { DCHECK(io_context_); static bool have_configured_service_worker_context = false; if (have_configured_service_worker_context) @@ -1505,6 +1508,9 @@ void NetworkHandler::GetCookies(Maybe> protocol_urls, void NetworkHandler::GetAllCookies( std::unique_ptr callback) { + if (!client_is_trusted_) { + callback->sendFailure(Response::ServerError(kNotAllowedError)); + } if (!storage_partition_) { callback->sendFailure(Response::InternalError()); return; diff --git a/chromium/content/browser/devtools/protocol/network_handler.h b/chromium/content/browser/devtools/protocol/network_handler.h index 6cbb0098e892..81636185d04f 100644 --- src/3rdparty/chromium/content/browser/devtools/protocol/network_handler.h +++ src/3rdparty/chromium/content/browser/devtools/protocol/network_handler.h @@ -72,7 +72,8 @@ class NetworkHandler : public DevToolsDomainHandler, const base::UnguessableToken& devtools_token, DevToolsIOContext* io_context, base::RepeatingClosure update_loader_factories_callback, - bool allow_file_access); + bool allow_file_access, + bool client_is_trusted); NetworkHandler(const NetworkHandler&) = delete; NetworkHandler& operator=(const NetworkHandler&) = delete; @@ -337,6 +338,8 @@ class NetworkHandler : public DevToolsDomainHandler, const base::UnguessableToken devtools_token_; DevToolsIOContext* const io_context_; + const bool allow_file_access_; + const bool client_is_trusted_; std::unique_ptr frontend_; BrowserContext* browser_context_; @@ -358,7 +361,6 @@ class NetworkHandler : public DevToolsDomainHandler, loaders_; absl::optional> accepted_stream_types_; - const bool allow_file_access_; std::unordered_map> received_body_data_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc index fe726068dee4..425eded3f56b 100644 --- src/3rdparty/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc +++ src/3rdparty/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -336,7 +336,8 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session, base::BindRepeating( &RenderFrameDevToolsAgentHost::UpdateResourceLoaderFactories, base::Unretained(this)), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); session->CreateAndAddHandler( GetIOContext(), base::BindRepeating( [](RenderFrameDevToolsAgentHost* self, diff --git a/chromium/content/browser/devtools/service_worker_devtools_agent_host.cc b/chromium/content/browser/devtools/service_worker_devtools_agent_host.cc index d2b307373ea1..7278a116ec78 100644 --- src/3rdparty/chromium/content/browser/devtools/service_worker_devtools_agent_host.cc +++ src/3rdparty/chromium/content/browser/devtools/service_worker_devtools_agent_host.cc @@ -230,7 +230,8 @@ bool ServiceWorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, session->CreateAndAddHandler(); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), base::DoNothing(), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); session->CreateAndAddHandler( GetIOContext(), diff --git a/chromium/content/browser/devtools/shared_worker_devtools_agent_host.cc b/chromium/content/browser/devtools/shared_worker_devtools_agent_host.cc index 6cfb49a9cb63..da9c8a3d18a4 100644 --- src/3rdparty/chromium/content/browser/devtools/shared_worker_devtools_agent_host.cc +++ src/3rdparty/chromium/content/browser/devtools/shared_worker_devtools_agent_host.cc @@ -91,7 +91,8 @@ bool SharedWorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, session->CreateAndAddHandler(); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), - base::BindRepeating([] {}), session->GetClient()->MayReadLocalFiles()); + base::BindRepeating([] {}), session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); // TODO(crbug.com/1143100): support pushing updated loader factories down to // renderer. session->CreateAndAddHandler( diff --git a/chromium/content/browser/devtools/worker_devtools_agent_host.cc b/chromium/content/browser/devtools/worker_devtools_agent_host.cc index 5bca24a4bb16..dbce6e066adb 100644 --- src/3rdparty/chromium/content/browser/devtools/worker_devtools_agent_host.cc +++ src/3rdparty/chromium/content/browser/devtools/worker_devtools_agent_host.cc @@ -137,7 +137,8 @@ bool WorkerDevToolsAgentHost::AttachSession(DevToolsSession* session, auto_attacher_.get(), session); session->CreateAndAddHandler( GetId(), devtools_worker_token_, GetIOContext(), base::DoNothing(), - session->GetClient()->MayReadLocalFiles()); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->IsTrusted()); return true; } From 9b72e2301892ea6619fb6e64f67812238ad56830 Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Mon, 18 Sep 2023 21:17:14 +0000 Subject: [PATCH] [Backport] Security bug 1407197 (1/2) Partial manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4869854: Tag WebContents ownership for debugging Tag WebContents owner and add it as a CrashKey for the DumpWithoutCrashing in ~WebContentsOfBrowserContext. The actual tags in this CL is more focused on android and is not exhaustive. Can keep adding new ones in the future as needed. Bug: 1407197 Change-Id: I6c0261ae5967fdb01ff2a5f3d0d6fe07f572bd20 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4869854 Reviewed-by: Ted Choc Commit-Queue: Bo Liu Reviewed-by: Avi Drissman Reviewed-by: Finnur Thorarinsson Cr-Commit-Position: refs/heads/main@{#1198010} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535707 Reviewed-by: Michal Klocek --- .../browser/distiller_page_web_contents.cc | 6 +++- .../guest_view/browser/guest_view_base.cc | 6 ++++ .../browser/no_state_prefetch_contents.cc | 1 + .../browser/no_state_prefetch_manager.cc | 5 +++ .../background_loader_contents.cc | 1 + chromium/content/browser/portal/portal.cc | 3 ++ chromium/content/browser/portal/portal.h | 3 ++ .../browser/web_contents/web_contents_impl.cc | 31 +++++++++++++++++-- .../browser/web_contents/web_contents_impl.h | 8 +++++ .../content/public/browser/web_contents.h | 6 ++++ chromium/extensions/browser/extension_host.cc | 3 +- 11 files changed, 69 insertions(+), 4 deletions(-) diff --git a/chromium/components/dom_distiller/content/browser/distiller_page_web_contents.cc b/chromium/components/dom_distiller/content/browser/distiller_page_web_contents.cc index e4025f7bc94c..78abc76a6bf2 100644 --- src/3rdparty/chromium/components/dom_distiller/content/browser/distiller_page_web_contents.cc +++ src/3rdparty/chromium/components/dom_distiller/content/browser/distiller_page_web_contents.cc @@ -30,7 +30,11 @@ namespace dom_distiller { SourcePageHandleWebContents::SourcePageHandleWebContents( content::WebContents* web_contents, bool owned) - : web_contents_(web_contents), owned_(owned) {} + : web_contents_(web_contents), owned_(owned) { + if (web_contents_ && owned) { + web_contents_->SetOwnerLocationForDebug(FROM_HERE); + } +} SourcePageHandleWebContents::~SourcePageHandleWebContents() { if (owned_) { diff --git a/chromium/components/guest_view/browser/guest_view_base.cc b/chromium/components/guest_view/browser/guest_view_base.cc index d2ea8b7ce3fd..06ba6ab1c7fc 100644 --- src/3rdparty/chromium/components/guest_view/browser/guest_view_base.cc +++ src/3rdparty/chromium/components/guest_view/browser/guest_view_base.cc @@ -480,6 +480,9 @@ void GuestViewBase::WillAttach( std::unique_ptr owned_guest_contents = std::move(owned_guest_contents_); DCHECK_EQ(owned_guest_contents.get(), web_contents()); + if (owned_guest_contents) { + owned_guest_contents->SetOwnerLocationForDebug(absl::nullopt); + } // Since this inner WebContents is created from the browser side we do // not have RemoteFrame mojo channels so we pass in @@ -774,6 +777,9 @@ void GuestViewBase::TakeGuestContentsOwnership( std::unique_ptr guest_web_contents) { DCHECK(!owned_guest_contents_); owned_guest_contents_ = std::move(guest_web_contents); + if (owned_guest_contents_) { + owned_guest_contents_->SetOwnerLocationForDebug(FROM_HERE); + } } void GuestViewBase::ClearOwnedGuestContents() { diff --git a/chromium/components/no_state_prefetch/browser/no_state_prefetch_contents.cc b/chromium/components/no_state_prefetch/browser/no_state_prefetch_contents.cc index f2f8dc5ff921..35fac905dc1f 100644 --- src/3rdparty/chromium/components/no_state_prefetch/browser/no_state_prefetch_contents.cc +++ src/3rdparty/chromium/components/no_state_prefetch/browser/no_state_prefetch_contents.cc @@ -271,6 +271,7 @@ void NoStatePrefetchContents::StartPrerendering( attempt_.get(), content::PreloadingTriggeringOutcome::kRunning); no_state_prefetch_contents_ = CreateWebContents(session_storage_namespace); + no_state_prefetch_contents_->SetOwnerLocationForDebug(FROM_HERE); content::WebContentsObserver::Observe(no_state_prefetch_contents_.get()); delegate_->OnNoStatePrefetchContentsCreated( no_state_prefetch_contents_.get()); diff --git a/chromium/components/no_state_prefetch/browser/no_state_prefetch_manager.cc b/chromium/components/no_state_prefetch/browser/no_state_prefetch_manager.cc index 3403fa8d1342..7397d1aa5de5 100644 --- src/3rdparty/chromium/components/no_state_prefetch/browser/no_state_prefetch_manager.cc +++ src/3rdparty/chromium/components/no_state_prefetch/browser/no_state_prefetch_manager.cc @@ -118,6 +118,7 @@ class NoStatePrefetchManager::OnCloseWebContentsDeleter OnCloseWebContentsDeleter(NoStatePrefetchManager* manager, std::unique_ptr tab) : manager_(manager), tab_(std::move(tab)) { + tab_->SetOwnerLocationForDebug(FROM_HERE); tab_->SetDelegate(this); base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask( FROM_HERE, @@ -140,6 +141,7 @@ class NoStatePrefetchManager::OnCloseWebContentsDeleter void ScheduleWebContentsForDeletion(bool timeout) { UMA_HISTOGRAM_BOOLEAN("Prerender.TabContentsDeleterTimeout", timeout); tab_->SetDelegate(nullptr); + tab_->SetOwnerLocationForDebug(absl::nullopt); manager_->ScheduleDeleteOldWebContents(std::move(tab_), this); // |this| is deleted at this point. } @@ -981,6 +983,9 @@ void NoStatePrefetchManager::CleanUpOldNavigations( void NoStatePrefetchManager::ScheduleDeleteOldWebContents( std::unique_ptr tab, OnCloseWebContentsDeleter* deleter) { + if (tab) { + tab->SetOwnerLocationForDebug(FROM_HERE); + } old_web_contents_list_.push_back(std::move(tab)); PostCleanupTask(); diff --git a/chromium/components/offline_pages/content/background_loader/background_loader_contents.cc b/chromium/components/offline_pages/content/background_loader/background_loader_contents.cc index e055852342ae..524b71f68bc0 100644 --- src/3rdparty/chromium/components/offline_pages/content/background_loader/background_loader_contents.cc +++ src/3rdparty/chromium/components/offline_pages/content/background_loader/background_loader_contents.cc @@ -23,6 +23,7 @@ BackgroundLoaderContents::BackgroundLoaderContents( // could kill the background offliner while it was running. web_contents_ = content::WebContents::Create( content::WebContents::CreateParams(browser_context_)); + web_contents_->SetOwnerLocationForDebug(FROM_HERE); web_contents_->SetAudioMuted(true); web_contents_->SetDelegate(this); } diff --git a/chromium/content/browser/portal/portal.cc b/chromium/content/browser/portal/portal.cc index f9b06fbab8df..b0c8b201db29 100644 --- src/3rdparty/chromium/content/browser/portal/portal.cc +++ src/3rdparty/chromium/content/browser/portal/portal.cc @@ -731,6 +731,9 @@ void Portal::WebContentsHolder::SetOwned( std::unique_ptr web_contents) { SetUnowned(static_cast(web_contents.get())); owned_contents_ = std::move(web_contents); + if (owned_contents_) { + owned_contents_->SetOwnerLocationForDebug(FROM_HERE); + } } void Portal::WebContentsHolder::Clear() { diff --git a/chromium/content/browser/portal/portal.h b/chromium/content/browser/portal/portal.h index 055d8e4f0cf5..aecf381ed594 100644 --- src/3rdparty/chromium/content/browser/portal/portal.h +++ src/3rdparty/chromium/content/browser/portal/portal.h @@ -177,6 +177,9 @@ class CONTENT_EXPORT Portal : public blink::mojom::Portal, // caller. std::unique_ptr ReleaseOwnership() { DCHECK(OwnsContents()); + if (owned_contents_) { + owned_contents_->SetOwnerLocationForDebug(absl::nullopt); + } return std::move(owned_contents_); } diff --git a/chromium/content/browser/web_contents/web_contents_impl.cc b/chromium/content/browser/web_contents/web_contents_impl.cc index 8b3f7055430c..d8b3ad83bbb6 100644 --- src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc +++ src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc @@ -795,6 +795,9 @@ void WebContentsImpl::WebContentsTreeNode::AttachInnerWebContents( inner_web_contents_node.outer_contents_frame_tree_node_id_ = render_frame_host->frame_tree_node()->frame_tree_node_id(); + if (inner_web_contents) { + inner_web_contents->SetOwnerLocationForDebug(FROM_HERE); + } inner_web_contents_.push_back(std::move(inner_web_contents)); render_frame_host->frame_tree_node()->AddObserver(&inner_web_contents_node); @@ -814,6 +817,9 @@ WebContentsImpl::WebContentsTreeNode::DetachInnerWebContents( std::swap(web_contents, inner_web_contents_.back()); inner_web_contents_.pop_back(); current_web_contents_->InnerWebContentsDetached(inner_web_contents); + if (detached_contents) { + detached_contents->SetOwnerLocationForDebug(absl::nullopt); + } return detached_contents; } } @@ -922,13 +928,29 @@ class WebContentsOfBrowserContext : public base::SupportsUserData::Data { // RenderFrameHosts, SiteInstances, etc.) risk causing // use-after-free bugs. For more discussion about managing the // lifetime of WebContents please see https://crbug.com/1376879#c44. - for (WebContents* web_contents_with_dangling_ptr_to_browser_context : + for (WebContentsImpl* web_contents_with_dangling_ptr_to_browser_context : web_contents_set_) { std::string creator = web_contents_with_dangling_ptr_to_browser_context ->GetCreatorLocation() .ToString(); SCOPED_CRASH_KEY_STRING256("shutdown", "web_contents/creator", creator); + const absl::optional& ownership_location = + web_contents_with_dangling_ptr_to_browser_context + ->ownership_location(); + std::string owner; + if (ownership_location) { + if (ownership_location->has_source_info()) { + owner = std::string(ownership_location->function_name()) + "@" + + ownership_location->file_name(); + } else { + owner = "no_source_info"; + } + } else { + owner = "unknown"; + } + SCOPED_CRASH_KEY_STRING256("shutdown", "web_contents/owner", owner); + #if BUILDFLAG(IS_ANDROID) // On Android, also report the Java stack trace from WebContents's // creation. @@ -974,7 +996,7 @@ class WebContentsOfBrowserContext : public base::SupportsUserData::Data { // Usage of `raw_ptr` below is okay (i.e. it shouldn't dangle), because // when `WebContentsImpl`'s destructor runs, then it removes the set entry // (by calling `Detach`). - std::set> web_contents_set_; + std::set> web_contents_set_; }; } // namespace @@ -9697,6 +9719,11 @@ std::unique_ptr WebContentsImpl::StartPrerendering( return nullptr; } +void WebContentsImpl::SetOwnerLocationForDebug( + absl::optional owner_location) { + ownership_location_ = owner_location; +} + void WebContentsImpl::AboutToBeDiscarded(WebContents* new_contents) { observers_.NotifyObservers(&WebContentsObserver::AboutToBeDiscarded, new_contents); diff --git a/chromium/content/browser/web_contents/web_contents_impl.h b/chromium/content/browser/web_contents/web_contents_impl.h index bc3dc3d00a39..815694c05b18 100644 --- src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.h +++ src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.h @@ -858,6 +858,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, PreloadingAttempt* preloading_attempt, absl::optional> url_match_predicate = absl::nullopt) override; + void SetOwnerLocationForDebug( + absl::optional owner_location) override; // NavigatorDelegate --------------------------------------------------------- @@ -1346,6 +1348,10 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, ui::mojom::VirtualKeyboardMode GetVirtualKeyboardMode() const; + const absl::optional& ownership_location() const { + return ownership_location_; + } + private: using FrameTreeIterationCallback = base::RepeatingCallback; using RenderViewHostIterationCallback = @@ -2370,6 +2376,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, base::WeakPtr active_file_chooser_; + absl::optional ownership_location_; + base::WeakPtrFactory loading_weak_factory_{this}; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/chromium/content/public/browser/web_contents.h b/chromium/content/public/browser/web_contents.h index bdd18c6c4ed8..6490fefaa940 100644 --- src/3rdparty/chromium/content/public/browser/web_contents.h +++ src/3rdparty/chromium/content/public/browser/web_contents.h @@ -1393,6 +1393,12 @@ class WebContents : public PageNavigator, absl::optional> url_match_predicate = absl::nullopt) = 0; + // Tag `WebContents` with its owner. Used purely for debugging purposes so it + // does not need to be exhaustive or perfectly correct. + // TODO(crbug.com/1407197): Remove after bug is fixed. + virtual void SetOwnerLocationForDebug( + absl::optional owner_location) = 0; + private: // This interface should only be implemented inside content. friend class WebContentsImpl; diff --git a/chromium/extensions/browser/extension_host.cc b/chromium/extensions/browser/extension_host.cc index 91928c8b7811..42c92939788b 100644 --- src/3rdparty/chromium/extensions/browser/extension_host.cc +++ src/3rdparty/chromium/extensions/browser/extension_host.cc @@ -63,7 +63,8 @@ ExtensionHost::ExtensionHost(const Extension* extension, host_type == mojom::ViewType::kExtensionPopup || host_type == mojom::ViewType::kExtensionSidePanel); host_contents_ = WebContents::Create( - WebContents::CreateParams(browser_context_, site_instance)), + WebContents::CreateParams(browser_context_, site_instance)); + host_contents_->SetOwnerLocationForDebug(FROM_HERE); content::WebContentsObserver::Observe(host_contents_.get()); host_contents_->SetDelegate(this); SetViewType(host_contents_.get(), host_type); From e257d6513927fa24df48075bce9b33c4b5f546ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Br=C3=BCning?= Date: Tue, 30 Jan 2024 11:51:16 +0100 Subject: [PATCH] [Backport] Security bug 1407197 (2/2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5080603: Safely crash on dangling profile Bug: 1407197 Change-Id: Idcafd8f0ba2f980d06338e573489a3456e3823c1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5080603 Reviewed-by: Łukasz Anforowicz Commit-Queue: Bo Liu Cr-Commit-Position: refs/heads/main@{#1232704} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/535708 Reviewed-by: Michal Klocek --- .../browser/web_contents/web_contents_impl.cc | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/chromium/content/browser/web_contents/web_contents_impl.cc b/chromium/content/browser/web_contents/web_contents_impl.cc index d8b3ad83bbb..3087f9c3e0b 100644 --- src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc +++ src/3rdparty/chromium/content/browser/web_contents/web_contents_impl.cc @@ -220,6 +220,11 @@ namespace { // The window which we dobounce load info updates in. constexpr auto kUpdateLoadStatesInterval = base::Milliseconds(250); +// Kill switch for crash immediately on dangling BrowserContext. +BASE_FEATURE(kCrashOnDanglingBrowserContext, + "CrashOnDanglingBrowserContext", + base::FEATURE_ENABLED_BY_DEFAULT); + using LifecycleState = RenderFrameHost::LifecycleState; using LifecycleStateImpl = RenderFrameHostImpl::LifecycleStateImpl; @@ -958,11 +963,18 @@ class WebContentsOfBrowserContext : public base::SupportsUserData::Data { env, web_contents_with_dangling_ptr_to_browser_context); #endif // BUILDFLAG(IS_ANDROID) - NOTREACHED() - << "BrowserContext is getting destroyed without first closing all " - << "WebContents (for more info see https://crbug.com/1376879#c44); " - << "creator = " << creator; - base::debug::DumpWithoutCrashing(); + if (base::FeatureList::IsEnabled(kCrashOnDanglingBrowserContext)) { + LOG(FATAL) + << "BrowserContext is getting destroyed without first closing all " + << "WebContents (for more info see https://crbug.com/1376879#c44); " + << "creator = " << creator; + } else { + NOTREACHED() + << "BrowserContext is getting destroyed without first closing all " + << "WebContents (for more info see https://crbug.com/1376879#c44); " + << "creator = " << creator; + base::debug::DumpWithoutCrashing(); + } } } +From f2480155fcf5f753d60b818986d136fcd2309edc Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= +Date: Tue, 23 Jan 2024 01:06:06 +0000 +Subject: [PATCH] [Backport] Security bug 1519980 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Manual cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5226127: +Speculatively fix race in mojo ShutDownOnIOThread + +This acquires `write_lock_` before resetting handles used by WriteNoLock +(which is called under the same lock in another thread). We also set +`reject_writes_` to prevent future write attempts after shutdown. That +seems strictly more correct. + +We also acquire `fds_to_close_lock_` before clearing the FDs. + +I was unable to repro locally as content_browsertests just times out +in my local setup without reporting anything interesting. This seems +strictly more correct though. + +Bug: 1519980 +Change-Id: I96279936ca908ecb98eddd381df20d61597cba43 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5226127 +Auto-Submit: Peter Boström +Reviewed-by: Ken Rockot +Commit-Queue: Ken Rockot +Commit-Queue: Peter Boström +Cr-Commit-Position: refs/heads/main@{#1250580} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537138 +Reviewed-by: Michal Klocek +--- + chromium/mojo/core/channel_posix.cc | 25 +++++++++++++++---------- + 1 file changed, 15 insertions(+), 10 deletions(-) + +diff --git a/chromium/mojo/core/channel_posix.cc b/chromium/mojo/core/channel_posix.cc +index f57c9b3cb5f..faf728fdd3d 100644 +--- src/3rdparty/chromium/mojo/core/channel_posix.cc ++++ src/3rdparty/chromium/mojo/core/channel_posix.cc +@@ -264,18 +264,23 @@ void ChannelPosix::WaitForWriteOnIOThreadNoLock() { + void ChannelPosix::ShutDownOnIOThread() { + base::CurrentThread::Get()->RemoveDestructionObserver(this); + +- read_watcher_.reset(); +- write_watcher_.reset(); +- if (leak_handle_) { +- std::ignore = socket_.release(); +- server_.TakePlatformHandle().release(); +- } else { +- socket_.reset(); +- std::ignore = server_.TakePlatformHandle(); +- } ++ { ++ base::AutoLock lock(write_lock_); ++ reject_writes_ = true; ++ read_watcher_.reset(); ++ write_watcher_.reset(); ++ if (leak_handle_) { ++ std::ignore = socket_.release(); ++ server_.TakePlatformHandle().release(); ++ } else { ++ socket_.reset(); ++ std::ignore = server_.TakePlatformHandle(); ++ } + #if BUILDFLAG(IS_IOS) +- fds_to_close_.clear(); ++ base::AutoLock fd_lock(fds_to_close_lock_); ++ fds_to_close_.clear(); + #endif ++ } + + // May destroy the |this| if it was the last reference. + self_ = nullptr; +From d9b4b11c104ec5112900dad72af8ff058c3f069b Mon Sep 17 00:00:00 2001 +From: Jean-Philippe Gravel +Date: Wed, 17 Jan 2024 17:45:45 +0000 +Subject: [PATCH] [Backport] CVE-2024-1060: Use after free in Canvas + +Manual backport of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5198419: +Fix use-after-free in DrawTextInternal + +DrawTextInternal was calling GetOrCreatePaintCanvas multiple times, +once at the start of the function, once inside of the +BaseRenderingContext2DAutoRestoreSkCanvas helper class and once in the +Draw call. GetOrCreatePaintCanvas destroys the canvas resource provider +if the GPU context is lost. If this happens on the second call to +GetOrCreatePaintCanvas, destroying the resource provider will +invalidate the cc::PaintCanvas returned by the first call to +GetOrCreatePaintCanvas. + +The GPU process can technically crash at any point during the renderer +process execution (perhaps because of something another renderer +process did). We therefore have to assume that any call to +GetOrCreatePaintCanvas can invalidate previously returned +cc::PaintCanvas. + +Change-Id: Ifa77735ab1b2b55b3d494f886b8566299937f6fe +Fixed: 1511567 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5198419 +Reviewed-by: Fernando Serboncini +Commit-Queue: Jean-Philippe Gravel +Cr-Commit-Position: refs/heads/main@{#1248204} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537140 +Reviewed-by: Michal Klocek +--- + .../canvas2d/canvas_rendering_context_2d.cc | 50 ++++++------------- + .../canvas2d/canvas_rendering_context_2d.h | 2 - + 2 files changed, 16 insertions(+), 36 deletions(-) + +diff --git a/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc b/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc +index 01720502d6a..adab5144f93 100644 +--- src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc ++++ src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc +@@ -102,35 +102,6 @@ static mojom::blink::ColorScheme GetColorSchemeFromCanvas( + return mojom::blink::ColorScheme::kLight; + } + +-// Drawing methods need to use this instead of SkAutoCanvasRestore in case +-// overdraw detection substitutes the recording canvas (to discard overdrawn +-// draw calls). +-class CanvasRenderingContext2DAutoRestoreSkCanvas { +- STACK_ALLOCATED(); +- +- public: +- explicit CanvasRenderingContext2DAutoRestoreSkCanvas( +- CanvasRenderingContext2D* context) +- : context_(context) { +- DCHECK(context_); +- cc::PaintCanvas* c = context_->GetOrCreatePaintCanvas(); +- if (c) { +- save_count_ = c->getSaveCount(); +- } +- } +- +- ~CanvasRenderingContext2DAutoRestoreSkCanvas() { +- cc::PaintCanvas* c = context_->GetOrCreatePaintCanvas(); +- if (c) +- c->restoreToCount(save_count_); +- context_->ValidateStateStack(); +- } +- +- private: +- CanvasRenderingContext2D* context_; +- int save_count_ = 0; +-}; +- + CanvasRenderingContext* CanvasRenderingContext2D::Factory::Create( + CanvasRenderingContextHost* host, + const CanvasContextCreationAttributesCore& attrs) { +@@ -999,9 +970,11 @@ void CanvasRenderingContext2D::DrawTextInternal( + // to 0, for example), so update style before grabbing the PaintCanvas. + canvas()->GetDocument().UpdateStyleAndLayoutTreeForNode(canvas()); + +- cc::PaintCanvas* c = GetOrCreatePaintCanvas(); +- if (!c) ++ // Abort if we don't have a paint canvas (e.g. the context was lost). ++ cc::PaintCanvas* paint_canvas = GetOrCreatePaintCanvas(); ++ if (!paint_canvas) { + return; ++ } + + if (!std::isfinite(x) || !std::isfinite(y)) + return; +@@ -1066,14 +1039,13 @@ void CanvasRenderingContext2D::DrawTextInternal( + if (paint_type == CanvasRenderingContext2DState::kStrokePaintType) + InflateStrokeRect(bounds); + +- CanvasRenderingContext2DAutoRestoreSkCanvas state_restorer(this); + if (use_max_width) { +- c->save(); ++ paint_canvas->save(); + // We draw when fontWidth is 0 so compositing operations (eg, a "copy" op) + // still work. As the width of canvas is scaled, so text can be scaled to + // match the given maxwidth, update text location so it appears on desired + // place. +- c->scale(ClampTo(width / font_width), 1); ++ paint_canvas->scale(ClampTo(width / font_width), 1); + location.set_x(location.x() / ClampTo(width / font_width)); + } + +@@ -1093,6 +1065,16 @@ void CanvasRenderingContext2D::DrawTextInternal( + { return false; }, + bounds, paint_type, CanvasRenderingContext2DState::kNoImage, + CanvasPerformanceMonitor::DrawType::kText); ++ ++ if (use_max_width) { ++ // Cannot use `paint_canvas` in case recording canvas was substituted or ++ // destroyed during draw call. ++ cc::PaintCanvas* c = GetPaintCanvas(); ++ if (c) { ++ c->restore(); ++ } ++ } ++ ValidateStateStack(); + } + + const Font& CanvasRenderingContext2D::AccessFont() { +diff --git a/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h b/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h +index 508af63e75a..59566cb117c 100644 +--- src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h ++++ src/3rdparty/chromium/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h +@@ -245,8 +245,6 @@ class MODULES_EXPORT CanvasRenderingContext2D final + void TryRestoreContextEvent(TimerBase*) override; + + private: +- friend class CanvasRenderingContext2DAutoRestoreSkCanvas; +- + void PruneLocalFontCache(size_t target_size); + + void ScrollPathIntoViewInternal(const Path&); +From 5f7b5772910e721f0cbdfd97925e84afa94aeec8 Mon Sep 17 00:00:00 2001 +From: Tsuyoshi Horo +Date: Tue, 9 Jan 2024 08:40:00 +0000 +Subject: [PATCH] [Backport] CVE-2024-1077: Use after free in Network + +Cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5179746: +Fix UAF in SourceStreamToDataPipe + +SourceStreamToDataPipe::ReadMore() is passing a callback with +Unretained(this) to net::SourceStream::Read(). But this callback may be +called even after the SourceStream is destructed. This is causing UAF +issue (crbug.com/1511085). + +To solve this problem, this CL changes ReadMore() method to pass a +callback with a weak ptr of this. + +Bug: 1511085 +Change-Id: Idd4e34ff300ff5db2de1de7b303841c7db3a964a +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5179746 +Reviewed-by: Adam Rice +Commit-Queue: Tsuyoshi Horo +Cr-Commit-Position: refs/heads/main@{#1244526} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537141 +Reviewed-by: Michal Klocek +--- + .../network/public/cpp/source_stream_to_data_pipe.cc | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/chromium/services/network/public/cpp/source_stream_to_data_pipe.cc b/chromium/services/network/public/cpp/source_stream_to_data_pipe.cc +index bfd85b1a00b..07afd58a40f 100644 +--- src/3rdparty/chromium/services/network/public/cpp/source_stream_to_data_pipe.cc ++++ src/3rdparty/chromium/services/network/public/cpp/source_stream_to_data_pipe.cc +@@ -55,9 +55,9 @@ void SourceStreamToDataPipe::ReadMore() { + + scoped_refptr buffer( + new network::NetToMojoIOBuffer(pending_write_.get())); +- int result = source_->Read( +- buffer.get(), base::checked_cast(num_bytes), +- base::BindOnce(&SourceStreamToDataPipe::DidRead, base::Unretained(this))); ++ int result = source_->Read(buffer.get(), base::checked_cast(num_bytes), ++ base::BindOnce(&SourceStreamToDataPipe::DidRead, ++ weak_factory_.GetWeakPtr())); + + if (result != net::ERR_IO_PENDING) + DidRead(result); +From 9bcf4d966b8315c3801721222c937f6c4fbc00b2 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Michael=20Br=C3=BCning?= +Date: Wed, 7 Feb 2024 12:07:44 +0100 +Subject: [PATCH] Fixup: [Backport] Security bug 1407197 + +It was missing setting one of the debug locations in code that we +may potentially compile. + +Change-Id: Ia47c270eb042d131621babaef3927b0745c36014 +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537953 +Reviewed-by: Michal Klocek +--- + chromium/chrome/browser/devtools/devtools_window.cc | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/chromium/chrome/browser/devtools/devtools_window.cc b/chromium/chrome/browser/devtools/devtools_window.cc +index de1b8b019fc..94343b63153 100644 +--- src/3rdparty/chromium/chrome/browser/devtools/devtools_window.cc ++++ src/3rdparty/chromium/chrome/browser/devtools/devtools_window.cc +@@ -1301,6 +1301,7 @@ void DevToolsWindow::AddNewContents( + bool* was_blocked) { + if (new_contents.get() == toolbox_web_contents_) { + owned_toolbox_web_contents_ = std::move(new_contents); ++ owned_toolbox_web_contents_->SetOwnerLocationForDebug(FROM_HERE); + + toolbox_web_contents_->SetDelegate(new DevToolsToolboxDelegate( + toolbox_web_contents_, inspected_web_contents_)); +From beb4a95a8040535701840e84338998b711cf86ff Mon Sep 17 00:00:00 2001 +From: Guido Urdaneta +Date: Thu, 18 Jan 2024 16:47:18 +0000 +Subject: [PATCH] [Backport] CVE-2024-1059: Use after free in WebRTC + +Manual backport of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5210359: +[RTCPeerConnection] Exit early from RTCPeerConnectionHandler + +For certain operations that require a live client +(i.e., RTCPeerConnection, which is garbage collected), +PeerConnectionHandler keeps a pointer to the client on the stack +to prevent garbage collection. + +In some cases, the client may have already been garbage collected +(the client is null). In that case, there is no point in doing the +operation and it should exit early to avoid UAF/crashes. + +This CL adds early exit to the cases that do not already have it. + +Bug: 1514777 +Change-Id: I27e9541cfaa74d978799c03e2832a0980f9e5710 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5210359 +Reviewed-by: Tomas Gunnarsson +Commit-Queue: Guido Urdaneta +Cr-Commit-Position: refs/heads/main@{#1248826} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/537139 +Reviewed-by: Michal Klocek +--- + .../rtc_peer_connection_handler.cc | 16 ++++++++++++---- + 1 file changed, 12 insertions(+), 4 deletions(-) + +diff --git a/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc b/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc +index 83853f003c7..fc2336dbb88 100644 +--- src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc ++++ src/3rdparty/chromium/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc +@@ -1058,15 +1058,19 @@ bool RTCPeerConnectionHandler::Initialize( + WebLocalFrame* frame, + ExceptionState& exception_state) { + DCHECK(task_runner_->RunsTasksInCurrentSequence()); +- DCHECK(frame); + DCHECK(dependency_factory_); +- frame_ = frame; + + CHECK(!initialize_called_); + initialize_called_ = true; + + // Prevent garbage collection of client_ during processing. + auto* client_on_stack = client_; ++ if (!client_on_stack) { ++ return false; ++ } ++ ++ DCHECK(frame); ++ frame_ = frame; + peer_connection_tracker_ = PeerConnectionTracker::From(*frame); + + configuration_ = server_configuration; +@@ -2268,10 +2272,13 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp, + int sdp_mline_index, + int component, + int address_family) { ++ DCHECK(task_runner_->RunsTasksInCurrentSequence()); + // In order to ensure that the RTCPeerConnection is not garbage collected + // from under the function, we keep a pointer to it on the stack. + auto* client_on_stack = client_; +- DCHECK(task_runner_->RunsTasksInCurrentSequence()); ++ if (!client_on_stack) { ++ return; ++ } + TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceCandidateImpl"); + // This line can cause garbage collection. + auto* platform_candidate = MakeGarbageCollected( +@@ -2281,7 +2288,8 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp, + this, platform_candidate, PeerConnectionTracker::kSourceLocal, true); + } + +- if (!is_closed_) ++ client_on_stack = client_; ++ if (!is_closed_ && client_on_stack) + client_on_stack->DidGenerateICECandidate(platform_candidate); + } + +From 149e8c185ff1ea7ee0a7037153311b026e142ac3 Mon Sep 17 00:00:00 2001 +From: John Stiles +Date: Mon, 29 Jan 2024 23:50:14 +0000 +Subject: [PATCH] [Backport] CVE-2024-1283: Heap buffer overflow in Skia + +Manual cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5241305: +Fix a crash when a BMP image contains an unnecessary EOF code. + +Previously, this would try to perform color correction on a row +one past the end of the image data. + +Bug: 1521893 +Change-Id: I425437005b9ef400138556705616095857d2cf0d +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5241305 +Auto-Submit: John Stiles +Commit-Queue: John Stiles +Reviewed-by: Peter Kasting +Cr-Commit-Position: refs/heads/main@{#1253633} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/538110 +Reviewed-by: Michal Klocek +--- + .../image-decoders/bmp/bmp_image_reader.cc | 17 ++++++++++++++--- + 1 file changed, 14 insertions(+), 3 deletions(-) + +diff --git a/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc b/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc +index 063e5385d7f6..b40c8aa5c1fe 100644 +--- src/3rdparty/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc ++++ src/3rdparty/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc +@@ -827,8 +827,10 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessRLEData() { + // the image. + const uint8_t count = ReadUint8(0); + const uint8_t code = ReadUint8(1); +- if ((count || (code != 1)) && PastEndOfImage(0)) ++ const bool is_past_end_of_image = PastEndOfImage(0); ++ if ((count || (code != 1)) && is_past_end_of_image) { + return kFailure; ++ } + + // Decode. + if (!count) { +@@ -849,7 +851,9 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessRLEData() { + (is_top_down_ ? (coord_.y() < (parent_->Size().height() - 1)) + : (coord_.y() > 0))) + buffer_->SetHasAlpha(true); +- ColorCorrectCurrentRow(); ++ if (!is_past_end_of_image) { ++ ColorCorrectCurrentRow(); ++ } + // There's no need to move |coord_| here to trigger the caller + // to call SetPixelsChanged(). If the only thing that's changed + // is the alpha state, that will be properly written into the +@@ -1061,6 +1065,13 @@ void BMPImageReader::ColorCorrectCurrentRow() { + const ColorProfileTransform* const transform = parent_->ColorTransform(); + if (!transform) + return; ++ int decoder_width = parent_->Size().width(); ++ // Enforce 0 ≤ current row < bitmap height. ++ CHECK_GE(coord_.y(), 0); ++ CHECK_LT(coord_.y(), buffer_->Bitmap().height()); ++ // Enforce decoder width == bitmap width exactly. (The bitmap rowbytes might ++ // add a bit of padding, but we are only converting one row at a time.) ++ CHECK_EQ(decoder_width, buffer_->Bitmap().width()); + ImageFrame::PixelData* const row = buffer_->GetAddr(0, coord_.y()); + const skcms_PixelFormat fmt = XformColorFormat(); + const skcms_AlphaFormat alpha = +@@ -1069,7 +1080,7 @@ void BMPImageReader::ColorCorrectCurrentRow() { + : skcms_AlphaFormat_Unpremul; + const bool success = + skcms_Transform(row, fmt, alpha, transform->SrcProfile(), row, fmt, alpha, +- transform->DstProfile(), parent_->Size().width()); ++ transform->DstProfile(), decoder_width); + DCHECK(success); + buffer_->SetPixelsChanged(true); + } +From 707f4e7c0110c33df3d36a1942ad1b0ea2cb997b Mon Sep 17 00:00:00 2001 +From: Ken Rockot +Date: Fri, 26 Jan 2024 21:53:06 +0000 +Subject: [PATCH] [Backport] CVE-2024-1284: Use after free in Mojo + +Cherry-pick of patch originally reviewed on +https://chromium-review.googlesource.com/c/chromium/src/+/5240312: +ipcz: Fix a few weak asserts + +DriverMemory cloning should not weakly assert success, as it can fail in +real production scenarios. Now Clone() will return an invalid +DriverMemory object if it fails to duplicate the internal handle. +Existing callers of Clone() are already durable to an invalid output, so +this change results in graceful failures instead of undefined behavior. + +This also replaces some weak asserts in DriverTransport creation with +hardening asserts. We may want to fail more gracefully if these end +up crashing a lot, but it seems unlikely. + +Fixed: 1521571 +Change-Id: Id764b33ead8bbba58e61b3270920c839479eaa4a +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5240312 +Commit-Queue: Ken Rockot +Reviewed-by: Alex Gough +Cr-Commit-Position: refs/heads/main@{#1252882} +Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/538111 +Reviewed-by: Michal Klocek +--- + .../third_party/ipcz/src/ipcz/driver_memory.cc | 15 +++++++++------ + .../third_party/ipcz/src/ipcz/driver_transport.cc | 4 ++-- + 2 files changed, 11 insertions(+), 8 deletions(-) + +diff --git a/chromium/third_party/ipcz/src/ipcz/driver_memory.cc b/chromium/third_party/ipcz/src/ipcz/driver_memory.cc +index 612eca89d52..b92c04bf521 100644 +--- src/3rdparty/chromium/third_party/ipcz/src/ipcz/driver_memory.cc ++++ src/3rdparty/chromium/third_party/ipcz/src/ipcz/driver_memory.cc +@@ -30,10 +30,11 @@ DriverMemory::DriverMemory(const IpczDriver& driver, size_t num_bytes) + : size_(num_bytes) { + ABSL_ASSERT(num_bytes > 0); + IpczDriverHandle handle; +- IpczResult result = ++ const IpczResult result = + driver.AllocateSharedMemory(num_bytes, IPCZ_NO_FLAGS, nullptr, &handle); +- ABSL_ASSERT(result == IPCZ_RESULT_OK); +- memory_ = DriverObject(driver, handle); ++ if (result == IPCZ_RESULT_OK) { ++ memory_ = DriverObject(driver, handle); ++ } + } + + DriverMemory::DriverMemory(DriverMemory&& other) = default; +@@ -43,12 +44,14 @@ DriverMemory& DriverMemory::operator=(DriverMemory&& other) = default; + DriverMemory::~DriverMemory() = default; + + DriverMemory DriverMemory::Clone() { +- ABSL_ASSERT(is_valid()); ++ ABSL_HARDENING_ASSERT(is_valid()); + + IpczDriverHandle handle; +- IpczResult result = memory_.driver()->DuplicateSharedMemory( ++ const IpczResult result = memory_.driver()->DuplicateSharedMemory( + memory_.handle(), 0, nullptr, &handle); +- ABSL_ASSERT(result == IPCZ_RESULT_OK); ++ if (result != IPCZ_RESULT_OK) { ++ return DriverMemory(); ++ } + + return DriverMemory(DriverObject(*memory_.driver(), handle)); + } +diff --git a/chromium/third_party/ipcz/src/ipcz/driver_transport.cc b/chromium/third_party/ipcz/src/ipcz/driver_transport.cc +index a8cb7a1251f..2550c2891fd 100644 +--- src/3rdparty/chromium/third_party/ipcz/src/ipcz/driver_transport.cc ++++ src/3rdparty/chromium/third_party/ipcz/src/ipcz/driver_transport.cc +@@ -68,14 +68,14 @@ DriverTransport::Pair DriverTransport::CreatePair( + IpczDriverHandle target_transport0 = IPCZ_INVALID_DRIVER_HANDLE; + IpczDriverHandle target_transport1 = IPCZ_INVALID_DRIVER_HANDLE; + if (transport0) { +- ABSL_ASSERT(transport1); ++ ABSL_HARDENING_ASSERT(transport1); + target_transport0 = transport0->driver_object().handle(); + target_transport1 = transport1->driver_object().handle(); + } + IpczResult result = driver.CreateTransports( + target_transport0, target_transport1, IPCZ_NO_FLAGS, nullptr, + &new_transport0, &new_transport1); +- ABSL_ASSERT(result == IPCZ_RESULT_OK); ++ ABSL_HARDENING_ASSERT(result == IPCZ_RESULT_OK); + auto first = + MakeRefCounted(DriverObject(driver, new_transport0)); + auto second =