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 4 changes from Release-1-M121 #41178

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 1511567.
  • Security: backported fix for 1514777.
  • Security: backported fix for 1511085.
  • Security: backported fix for 1519980.

@codebytere codebytere requested a review from a team as a code owner January 30, 2024 19:54
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jan 30, 2024
@codebytere codebytere added backport This is a backport PR semver/patch backwards-compatible bug fixes backport-check-skip Skip trop's backport validity checking 28-x-y labels Jan 30, 2024
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jan 30, 2024
@codebytere codebytere changed the title chore: [28-x-y] cherry-pick 3 changes from Release-1-M121 chore: cherry-pick 3 changes from Release-1-M121 Jan 31, 2024
@codebytere codebytere force-pushed the cherry-pick/security/28-x-y/release-1-m121 branch from adf5181 to 225cc59 Compare January 31, 2024 09:43
@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.

@codebytere codebytere added security 🔒 and removed backport This is a backport PR labels Feb 1, 2024
* d4a197e4913f from chromium
* 8755f76bec32 from chromium
* e321f354a613 from chromium
* 4a98f9e304be from chromium
@codebytere codebytere force-pushed the cherry-pick/security/28-x-y/release-1-m121 branch from 225cc59 to 7e30f11 Compare February 1, 2024 17:34
@ppontes ppontes changed the title chore: cherry-pick 3 changes from Release-1-M121 chore: cherry-pick 4 changes from Release-1-M121 Feb 2, 2024
@codebytere codebytere merged commit ace362c into 28-x-y Feb 2, 2024
15 checks passed
@codebytere codebytere deleted the cherry-pick/security/28-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 1511567.
  • Security: backported fix for 1514777.
  • Security: backported fix for 1511085.
  • Security: backported fix for 1519980.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
28-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