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.

Sign up for GitHub

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 fix from chromium issue 1080481 #24587

Merged
merged 1 commit into from
Jul 16, 2020
Merged

chore: cherry-pick fix from chromium issue 1080481 #24587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion patches/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@

"src/electron/patches/pdfium": "src/third_party/pdfium",

"src/electron/patches/webrtc": "src/third_party/webrtc"
"src/electron/patches/webrtc": "src/third_party/webrtc",

"src/electron/patches/skia": "src/third_party/skia"
}
1 change: 1 addition & 0 deletions patches/skia/.patches
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
backport_1080481.patch
88 changes: 88 additions & 0 deletions patches/skia/backport_1080481.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Thu, 4 Oct 2018 14:57:02 -0700
Subject: fix: drop SkTextBlobs with > 2M glyphs

[1080481] [Medium] [CVE-2020-6523]: Security: Skia: Integer Overflow in GrTextBlob::Make
Backport https://skia.googlesource.com/skia.git/+/8d2ebfffaf6ece9a7e9839dca2d7907f241c3460

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 9dd0f16b0b69f3a2f1c592bb0a3755c51fa3a099..3b4a9735d4615591a48b8f7578e3449de2c17c07 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -2697,6 +2697,19 @@ void SkCanvas::drawTextBlob(const SkTextBlob* blob, SkScalar x, SkScalar y,
TRACE_EVENT0("skia", TRACE_FUNC);
RETURN_ON_NULL(blob);
RETURN_ON_FALSE(blob->bounds().makeOffset(x, y).isFinite());
+
+ // Overflow if more than 2^21 glyphs stopping a buffer overflow latter in the stack.
+ // See chromium:1080481
+ // TODO: can consider unrolling a few at a time if this limit becomes a problem.
+ int totalGlyphCount = 0;
+ constexpr int kMaxGlyphCount = 1 << 21;
+ SkTextBlob::Iter i(*blob);
+ SkTextBlob::Iter::Run r;
+ while (i.next(&r)) {
+ int glyphsLeft = kMaxGlyphCount - totalGlyphCount;
+ RETURN_ON_FALSE(r.fGlyphCount <= glyphsLeft);
+ totalGlyphCount += r.fGlyphCount;
+ }
this->onDrawTextBlob(blob, x, y, paint);
}

diff --git a/tests/TextBlobCacheTest.cpp b/tests/TextBlobCacheTest.cpp
index bcc7288a2afe800706bc924eea5050786347c747..3508221408c801957dfc8d4cb52c6ac3ddd08a77 100644
--- a/tests/TextBlobCacheTest.cpp
+++ b/tests/TextBlobCacheTest.cpp
@@ -229,6 +229,51 @@ static sk_sp<SkTextBlob> make_blob() {
return builder.make();
}

+// Turned off to pass on android and ios devices, which were running out of memory..
+#if 0
+static sk_sp<SkTextBlob> make_large_blob() {
+ auto tf = SkTypeface::MakeFromName("Roboto2-Regular", SkFontStyle());
+ SkFont font;
+ font.setTypeface(tf);
+ font.setSubpixel(false);
+ font.setEdging(SkFont::Edging::kAlias);
+ font.setSize(24);
+
+ const int mallocSize = 0x3c3c3bd; // x86 size
+ std::unique_ptr<char[]> text{new char[mallocSize + 1]};
+ if (text == nullptr) {
+ return nullptr;
+ }
+ for (int i = 0; i < mallocSize; i++) {
+ text[i] = 'x';
+ }
+ text[mallocSize] = 0;
+
+ static const int maxGlyphLen = mallocSize;
+ std::unique_ptr<SkGlyphID[]> glyphs{new SkGlyphID[maxGlyphLen]};
+ int glyphCount =
+ font.textToGlyphs(
+ text.get(), mallocSize, SkTextEncoding::kUTF8, glyphs.get(), maxGlyphLen);
+ SkTextBlobBuilder builder;
+ const auto& runBuffer = builder.allocRun(font, glyphCount, 0, 0);
+ for (int i = 0; i < glyphCount; i++) {
+ runBuffer.glyphs[i] = glyphs[i];
+ }
+ return builder.make();
+}
+
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TextBlobIntegerOverflowTest, reporter, ctxInfo) {
+ auto grContext = ctxInfo.grContext();
+ const SkImageInfo info =
+ SkImageInfo::Make(kScreenDim, kScreenDim, kN32_SkColorType, kPremul_SkAlphaType);
+ auto surface = SkSurface::MakeRenderTarget(grContext, SkBudgeted::kNo, info);
+
+ auto blob = make_large_blob();
+ int y = 40;
+ SkBitmap base = draw_blob(blob.get(), surface.get(), {40, y + 0.0f});
+}
+#endif
+
static const bool kDumpPngs = true;
// dump pngs needs a "good" and a "bad" directory to put the results in. This allows the use of the
// skdiff tool to visualize the differences.