CVE-2026-9983
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
whilesrc/pathops/SkOpCoincidence.cpp |
modified | |
ifsrc/pathops/SkOpCoincidence.cpp |
modified |
Files Changed
src/pathops/SkOpCoincidence.cpp
Patch
From 33b70be027e80ccc2e1c36dd60db32791a7f34ef Mon Sep 17 00:00:00 2001
From: Kaylee Lubick <kjlubick@google.com>
Date: Mon, 18 May 2026 16:07:23 +0000
Subject: [PATCH] Fix pathops bug with linked lists in SkOpCoincidence
I added some logging to SkOpCoincidence::fixUp and ::release and,
with the new test case (which has multiple coincidences [1]), observed
```
fixUp: release(0x...7378, 0x...7378)
release: head_arg=0x...7378, remove=0x...7378, global fHead=...7378
release: updated global fHead to 0x...72b8
fixUp: after release global fHead=0x...72b8
fixUp: release(0x...7378, 0x...6ec8)
```
As per the linked bug, this leads to a faulty state because the
old head pointer 0x...7378 is being used after it was "released".
I could not get ASAN to fire on this but it is worth fixing for
correctness and consistency.
My change fixes that particular issue and adds some asserts to avoid
this happening again.
I also rewrote some do while loops to
avoid iffy behavior where we were releasing coin and then for
the next iteration still calling coin->next(). This worked because
we had an SkArenaAlloc and that memory wasn't "fully" released.
[1] a coincidence is an overlap between two path segments that
lasts for more than a single point. e.g. two squares sharing an edge.
Bug: 513001309
Fixed: 513001309
Change-Id: Ie3f6cea575acf9575e984b2b0d2eb788bb4b2052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1238176
Reviewed-by: Thomas Smith <thomsmit@google.com>
Commit-Queue: Kaylee Lubick <kjlubick@google.com>
---
diff --git a/src/pathops/SkOpCoincidence.cpp b/src/pathops/SkOpCoincidence.cpp
index 4a8bcec..a8a604e 100644
--- a/src/pathops/SkOpCoincidence.cpp
+++ b/src/pathops/SkOpCoincidence.cpp
@@ -695,8 +695,8 @@
: overlap->oppPtTEnd()->fT < test->oppPtTEnd()->fT) {
overlap->setOppPtTEnd(test->oppPtTEnd());
}
- if (!fHead || !this->release(fHead, test)) {
- SkAssertResult(this->release(fTop, test));
+ if (!fHead || !this->release(&fHead, test)) {
+ SkAssertResult(this->release(&fTop, test));
}
}
const SkOpPtT* cs = coinSeg->existing(coinTs, oppSeg);
@@ -1157,57 +1157,54 @@
}
// Please keep this in sync with debugRelease()
-bool SkOpCoincidence::release(SkCoincidentSpans* coin, SkCoincidentSpans* remove) {
- SkCoincidentSpans* head = coin;
+bool SkOpCoincidence::release(SkCoincidentSpans** headPtr, SkCoincidentSpans* remove) {
+ SkASSERT(headPtr == &fHead || headPtr == &fTop);
+ SkCoincidentSpans* coin = *headPtr;
SkCoincidentSpans* prev = nullptr;
SkCoincidentSpans* next;
- do {
+ while (coin) {
next = coin->next();
if (coin == remove) {
if (prev) {
prev->setNext(next);
- } else if (head == fHead) {
- fHead = next;
} else {
- fTop = next;
+ *headPtr = next;
}
break;
}
prev = coin;
- } while ((coin = next));
+ coin = next;
+ }
return coin != nullptr;
}
-void SkOpCoincidence::releaseDeleted(SkCoincidentSpans* coin) {
- if (!coin) {
- return;
- }
- SkCoincidentSpans* head = coin;
+void SkOpCoincidence::releaseDeleted(SkCoincidentSpans** headPtr) {
+ SkASSERT(headPtr == &fHead || headPtr == &fTop);
+ SkCoincidentSpans* coin = *headPtr;
SkCoincidentSpans* prev = nullptr;
SkCoincidentSpans* next;
- do {
+ while (coin) {
next = coin->next();
if (coin->coinPtTStart()->deleted()) {
SkOPASSERT(coin->flipped() ? coin->oppPtTEnd()->deleted() :
coin->oppPtTStart()->deleted());
if (prev) {
prev->setNext(next);
- } else if (head == fHead) {
- fHead = next;
} else {
- fTop = next;
+ *headPtr = next;
}
} else {
SkOPASSERT(coin->flipped() ? !coin->oppPtTEnd()->deleted() :
!coin->oppPtTStart()->deleted());
prev = coin;
}
- } while ((coin = next));
+ coin = next;
+ }
}
void SkOpCoincidence::releaseDeleted() {
- this->releaseDeleted(fHead);
- this->releaseDeleted(fTop);
+ this->releaseDeleted(&fHead);
+ this->releaseDeleted(&fTop);
}
void SkOpCoincidence::restoreHead() {
@@ -1248,7 +1245,7 @@
}
if (coin->coinPtTStart() == test->coinPtTStart()
&& coin->oppPtTStart() == test->oppPtTStart()) {
- this->release(fHead, test);
+ this->release(&fHead, test);
break;
}
} while ((test = test->next()));
@@ -1297,45 +1294,52 @@
void SkOpCoincidence::fixUp(SkOpPtT* deleted, const SkOpPtT* kept) {
SkOPASSERT(deleted != kept);
if (fHead) {
- this->fixUp(fHead, deleted, kept);
+ this->fixUp(&fHead, deleted, kept);
}
if (fTop) {
- this->fixUp(fTop, deleted, kept);
+ this->fixUp(&fTop, deleted, kept);
}
}
-void SkOpCoincidence::fixUp(SkCoincidentSpans* coin, SkOpPtT* deleted, const SkOpPtT* kept) {
- SkCoincidentSpans* head = coin;
- do {
+void SkOpCoincidence::fixUp(SkCoincidentSpans** headPtr, SkOpPtT* deleted, const SkOpPtT* kept) {
+ SkASSERT(headPtr == &fHead || headPtr == &fTop);
+ SkCoincidentSpans* coin = *headPtr;
+ while (coin) {
+ SkCoincidentSpans* next = coin->next();
if (coin->coinPtTStart() == deleted) {
if (coin->coinPtTEnd()->span() == kept->span()) {
- this->release(head, coin);
+ this->release(headPtr, coin);
+ coin = next;
continue;
}
coin->setCoinPtTStart(kept);
}
if (coin->coinPtTEnd() == deleted) {
if (coin->coinPtTStart()->span() == kept->span()) {
- this->release(head, coin);
+ this->release(headPtr, coin);
+ coin = next;
continue;
}
coin->setCoinPtTEnd(kept);
}
if (coin->oppPtTStart() == deleted) {
if (coin->oppPtTEnd()->span() == kept->span()) {
- this->release(head, coin);
+ this->release(headPtr, coin);
+ coin = next;
continue;
}
coin->setOppPtTStart(kept);
}
if (coin->oppPtTEnd() == deleted) {
if (coin->oppPtTStart()->span() == kept->span()) {
- this->release(head, coin);
+ this->release(headPtr, coin);
+ coin = next;
continue;
}
coin->setOppPtTEnd(kept);
}
- } while ((coin = coin->next()));
+ coin = next;
+ }
}
Original Bug Report
Logic error in SkOpCoincidence list management allows released spans to remain reachable
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A logic error in Skia’s PathOps module causes coincidence lists to be incorrectly updated when multiple consecutive elements are removed. This results in ‘released’ spans remaining reachable via the list head, potentially leading to intra-arena type confusion and memory corruption during path processing.
Affected files:
third_party/skia/src/pathops/SkOpCoincidence.cppthird_party/skia/src/pathops/SkOpSpan.cpp
Estimated timestamp from git blame: 2016-07-18
Summary
A logic error exists in SkOpCoincidence::fixUp and SkOpCoincidence::markCollapsed within Skia’s PathOps module. These functions fail to update a local reference to the coincidence list head after removing the first element. If subsequent elements also require removal, the unlinking logic operates on a stale pointer, leaving deleted nodes reachable via the list head (fHead or fTop).
Root Cause Analysis
In SkOpCoincidence::fixUp and SkOpCoincidence::markCollapsed, the head of the list is captured into a local variable head before iterating through and modifying the list:
// third_party/skia/src/pathops/SkOpCoincidence.cpp:1307
void SkOpCoincidence::fixUp(SkCoincidentSpans* coin, SkOpPtT* deleted, const SkOpPtT* kept) {
SkCoincidentSpans* head = coin; // Captured once
do {
if (coin->coinPtTStart() == deleted) {
if (coin->coinPtTEnd()->span() == kept->span()) {
this->release(head, coin); // Passes STALE head if a previous node was removed
continue;
}
// ...
}
} while ((coin = coin->next()));
}
The release(SkCoincidentSpans* head, SkCoincidentSpans* remove) helper uses the provided head pointer to walk the list and perform the unlinking. If release removes the current head of the list, it updates the member variable fHead. However, the caller’s local variable head remains pointed at the old, now unlinked node.
If a subsequent node also matches the deletion criteria, the next call to release uses the stale head. The walk inside release starts at the unlinked node, finds the target node, and updates the next pointer of the unlinked node instead of updating fHead. This results in the target node remaining reachable via the actual fHead member.
Potential Impact
This vulnerability allows a “released” SkCoincidentSpans node to remain reachable in the coincidence list while containing pointers to deleted or merged span data.
Subsequent PathOps phases (e.g., correctEnds(), expand()) iterate this list and perform upCast() operations on the spans. If a degenerate span refers to a segment tail (SkOpSpanBase), the upCast() to SkOpSpan is invalid. In release builds where SkASSERT is disabled, this results in intra-arena type confusion, allowing out-of-bounds reads or writes to adjacent objects within the SkArenaAlloc used for the Path operation.
Suggested Potential Reproduction Steps
- Define a path with multiple overlapping (coincident) segments that share a common vertex.
- Perform a Path operation (e.g.,
SimplifyorOp) that triggers the merging of the shared vertex. - Ensure the coincidence list contains at least two consecutive nodes that will be released during the
fixUpphase. - The logic error should leave the second node reachable via
fHeaddespite its intended removal. - The vulnerability is reachable from the renderer process via attacker-controlled path geometry in Canvas2D or SVG.
Recommended Fix
Modify the release helper to take a pointer-to-pointer (SkCoincidentSpans** headPtr) or ensure that the calling functions update their local head pointer after a successful release. Using a pointer-to-pointer allows release to directly update the caller’s state and correctly manage the list head update regardless of which list (fHead or fTop) is being processed.
Evaluated with Chrome root at commit: b3153093eb3c78c3e88ccf562bcbc20437a04b0e
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.