Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 3 changes from Release-1-M121 #41177

Merged
merged 2 commits into from Feb 2, 2024

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Jan 30, 2024

electron/security#460 - d4a197e4913f from chromium 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 fserb@chromium.org
Commit-Queue: Jean-Philippe Gravel jpgravel@chromium.org
Cr-Commit-Position: refs/heads/main@{#1248204}

electron/security#459 - 8755f76bec32 from chromium [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 tommi@chromium.org
Commit-Queue: Guido Urdaneta guidou@chromium.org
Cr-Commit-Position: refs/heads/main@{#1248826}

electron/security#461 - e321f354a613 from chromium [M121] 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.

(cherry picked from commit 6e36a69da1b73f9aea9c54bfbe6c5b9cb2c672a5)

Bug: 1511085
Change-Id: Idd4e34ff300ff5db2de1de7b303841c7db3a964a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5179746
Reviewed-by: Adam Rice ricea@chromium.org
Commit-Queue: Tsuyoshi Horo horo@chromium.org
Cr-Original-Commit-Position: refs/heads/main@{#1244526}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5231537
Reviewed-by: Kenichi Ishibashi bashi@chromium.org
Cr-Commit-Position: refs/branch-heads/6167@{#1621}
Cr-Branched-From: 222e786949e76e342d325ea0d008b4b6273f3a89-refs/heads/main@{#1233107}

Notes:

  • Security: backported fix for 1519980.
  • Security: backported fix for 1514777.
  • Security: backported fix for 1511085.

@codebytere codebytere requested a review from a team as a code owner January 30, 2024 19:51
@codebytere codebytere added security 🔒 semver/patch backwards-compatible bug fixes backport-check-skip Skip trop's backport validity checking 26-x-y labels Jan 30, 2024
@ppontes
Copy link
Member

ppontes commented Jan 31, 2024

@codebytere , I think we should also include 5239564: Speculatively fix race in mojo ShutDownOnIOThread, which is the fix to the embargoed critical 1519980.

@ppontes
Copy link
Member

ppontes commented Jan 31, 2024

Regarding 1511567, the vulnerability was introduced by 4780556: Move more text rendering code into BaseRenderingContext2D, which affected M118 the earliest. So that backport can be skipped for E26.

* ee0b8769f428 from chromium
* 1f8bec968902 from chromium
* 4a98f9e304be from chromium
@codebytere codebytere force-pushed the cherry-pick/security/26-x-y/release-1-m121 branch from dd69d91 to 32bd7da Compare February 1, 2024 18:22
@codebytere codebytere merged commit 1390874 into 26-x-y Feb 2, 2024
14 checks passed
@codebytere codebytere deleted the cherry-pick/security/26-x-y/release-1-m121 branch February 2, 2024 11:33
Copy link

release-clerk bot commented Feb 2, 2024

Release Notes Persisted

  • Security: backported fix for 1519980.
  • Security: backported fix for 1514777.
  • Security: backported fix for 1511085.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
26-x-y backport-check-skip Skip trop's backport validity checking security 🔒 semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants