Skip to content

Commit

Permalink
chore: cherry-pick fix from chromium issue 1080481
Browse files Browse the repository at this point in the history
  • Loading branch information
zcbenz committed Jul 16, 2020
1 parent c20701b commit c083b0a
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 1 deletion.
4 changes: 3 additions & 1 deletion patches/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@

"src/electron/patches/usrsctp": "src/third_party/usrsctp/usrsctplib",

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

"src/electron/patches/skia": "src/third_party/skia"
}
1 change: 1 addition & 0 deletions patches/skia/.patches
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
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.

0 comments on commit c083b0a

Please sign in to comment.