Closed Bug 1642028 (CVE-2020-16012) Opened 11 months ago Closed 6 months ago

drawImage timing depends on alpha-channel value, allowing to read cross-origin images

Categories

(Core :: Canvas: 2D, task)

task
Not set
normal

Tracking

()

VERIFIED FIXED
83 Branch
Tracking Status
firefox-esr78 83+ fixed
firefox82 --- wontfix
firefox83 + verified

People

(Reporter: aleksejs, Assigned: lsalzman)

References

Details

(Keywords: csectype-sop, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage][adv-main83+][adv-esr78.5+])

Attachments

(3 files, 1 obsolete file)

Attached file drawimage-timing.zip

Greetings,

I have discovered a timing side-channel that can be used recover parts of images located on 3rd party origins.

At least on my machine, calling drawImage(img, 0, 0, 1, 1, 0, 0, 1024, 1024) on a CanvasRenderingContext2D (which draws the top-left pixel of img onto the canvas, blown up to be 1024x1024) takes a measurably shorter amount of time when the top-left pixel is completely transparent (A = 0) than when it is completely opaque (A = 255), which in turn takes a measurably shorter time than when it is partially opaque (1 <= A <= 254).

Because img can be an image from a different origin, this can be used to figure out which of its pixels are transparent, thus inferring its outline.

I am attaching an archive that contains two proofs of concept:

  • in the folder "benchmark" is a page that measures the performance of drawImage using source images of different colors and outputs a CSV (warning: it will lock up the renderer for a couple minutes). "drawimage_timing.png" is a chart generated from the output of this benchmark on my machine, showing clearly the three different performance levels.
  • in the folder "exploit" is a complete exploit that will reconstruct a 5x5 remote image in about 10s. Note that opening it will make a remote request to my server to load the target image. "exploit_screenshot.png" is a screenshot showing it working successfully on my machine.

I am not sure how to make a PoC that would be easily integrated into a test suite, and am open to suggestions. Perhaps a version of "benchmark" that also runs a statistical test on its output could be helpful?

I have tested this on an empty profile on Firefox 76.0.1 64-bit, running on Linux 5.6.14 using X. As far as I can tell, here this behavior is ultimately caused by optimizations in Cairo, the graphics library used on this platform:

  • CanvasRenderingContext2D::drawImage calls DrawTargetCairo::DrawSurface (gfx/2d/DrawTargetCairo.cpp),
  • which calls PaintWithAlpha (gfx/2d/DrawTargetCairo.cpp),
  • which calls cairo_paint (gfx/cairo/cairo/src/cairo.c),
  • which calls _cairo_gstate_paint (gfx/cairo/cairo/src/cairo-gstate.c),
  • which calls _cairo_surface_paint (gfx/cairo/cairo/src/cairo-surface.c),
  • which calls _cairo_pattern_is_clear in the same file. This function returns TRUE if the source is fully transparent, in which case _cairo_surface_paint returns and we get the super-fast behavior for A = 0.

I am not yet sure where the difference in behavior in the 1 <= A <= 254 vs A = 255 cases is coming from, though that also sounds like a reasonable optimization a graphics library might perform.

I imagine similar behavior might be present on other platforms using other graphics libraries, but don't have a machine running a different OS to check. The benchmarking code I include should hopefully be helpful for diagnosing that.

Flags: sec-bounty?
Group: firefox-core-security → gfx-core-security
Component: Security → Canvas: 2D
Product: Firefox → Core

My apologies, the machine hosting the target image for the exploit demo is down :( If you want to see the exploit in action, just replace the src attribute on line 21 of exploit/index.html with https://p.2038.io/target_xsmall.png

As a heads up, I have also reported a similar bug to the Chrome team.

Can you give us the link to the chrome bug or at least the number? I realize their bug will be inaccessible but it's good to have the reference for later and also so we can talk to them about it if necessary.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aleksejs)
Flags: needinfo?(lsalzman)

Sure, it's https://bugs.chromium.org/p/chromium/issues/detail?id=1088224 (indeed, not publicly accessible for now)

Flags: needinfo?(aleksejs)

We don't enable Cairo at all for any users by default. It's hidden behind a pref, and that pref only works on Linux. It's either Direct2D, or if not available (due to platform or unsupported), then Skia is otherwise always the default.

Flags: needinfo?(lsalzman)

My bad, looks like I diagnosed this wrong. I saw the property CairoUseXRender in about:support and assumed that must mean Firefox was using Cairo, but I now see that the AzureCanvasBackend and AzureContentBackend properties are both set to skia. Let me see if I can track down the code in Skia that is causing this.

Here's one place where Skia looks like it should have different performance for opaque vs non-opaque images:

DrawTragetSkia::DrawSurface → SkCanvas::drawImageRect → SkCanvas::onDrawImageRect. Through the macro DRAW_BEGIN_CHECK_COMPLETE_OVERWRITE, onDrawImageRect will pass the value of image->isOpaque() into SkCanvas::predrawNotify, which uses it to determine (lines 158-167 of gfx/skia/skia/src/core/SkCanvas.cpp in the 76.0.1 source tarball) whether to discard the surface.

You'd think that this means we should see worse performance if the surface cannot be discarded even on opaque draws, such as if I make the canvas slightly bigger than 1024×1024. But performance doesn't seem to change if I do that, so I'm not sure what's going on.

There are also other places where the isOpaque check is used, though, like SkBitmapScaler.

It seems that this behavior is likely caused by the short-circuiting optimizations in the function blit_row_s32a_opaque() in gfx/skia/skia/src/opts/SkBlitRow_opts.h.

Any updates on your side about the bug and/or bounty eligibility?

Do you have a proof of concept that actually shows the Skia path being affected, not just theoretically, but measurably in a profile?

The exploit attached to my original submission works on a default profile, which, as I understand from your earlier comment, runs Skia. Is there anything else you want me to demonstrate?

I would appreciate a profile from the Firefox profiler showing that it is in fact spending time where you believe it to be. That would help in making a further decision about what to do.

Ah, gotcha! Sorry, the ambiguity of the word “profile” confused me :)

Here's a link to a profile I just recorded while running the benchmark I previously attached (with the set of source images restricted to just three, rgba-0-0-0-{0,128,255}): https://share.firefox.dev/3fNsFEg

If I'm reading the flame graph right, looks like Skia really is spending basically all of the time in blit_row_s32a_opaque, and there are three different stacks that end in that function, corresponding to the three different source images, with the runtime of blit_row_s32a_opaque varying a lot depending on which image is used.

Looks like the fix will go out in Chrome in November, and they expect to disclose the details of the bug in December.

Would you like somebody from Mozilla to be CCd on the Chrome bug?

(In reply to Aleksejs Popovs from comment #14)

Looks like the fix will go out in Chrome in November, and they expect to disclose the details of the bug in December.

Would you like somebody from Mozilla to be CCd on the Chrome bug?

I would like to be CC'd.

Lee, do we have a separate bug for updating Skia or are we going to repurpose this one?

Flags: needinfo?(lsalzman)
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED

Just cherry-picking the relevant cleanup here from https://chromium.googlesource.com/skia/+/5d3314c53ce5c966591f0b02349103f51f986e6e, as this wouldn't necessitate a Skia update by any stretch of the imagination.

Flags: needinfo?(lsalzman)
Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify-
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage]
Flags: qe-verify- → qe-verify+

Considering the "qe-verify+" tag, I have taken this bug to attempt to reproduce and verify. I have tried to see the difference between the affected and unaffected builds and these were my results:

  • Ubuntu 20.04LTS, Nightly v83.0a1 from 2020-10-01:
    • both pages took quite a long time to show the results
      • benchmark folded index.html :
        rgba-0-0-0-0 116
        110 123 110
        108 114 125
      • exploit folder index.html :
        the reconstruction is very similar to the original
  • Ubuntu 20.04LTS, Nightly v83.0a1 from 2020-10-18:
    • both pages took quite a long time to show the results
      • benchmark folded index.html :
        rgba-0-0-0-0 932
        932 1132 958
        936 938 938
      • exploit folder index.html :
        the reconstruction is very similar to a cube than the original

I do not know what the expected results would be or how I should properly verify this fix.
Lee, Can you help me verify it?

Flags: needinfo?(lsalzman)

(In reply to Bodea Daniel [:danibodea] from comment #20)

Considering the "qe-verify+" tag, I have taken this bug to attempt to reproduce and verify. I have tried to see the difference between the affected and unaffected builds and these were my results:

  • Ubuntu 20.04LTS, Nightly v83.0a1 from 2020-10-01:
    • both pages took quite a long time to show the results
      • benchmark folded index.html :
        rgba-0-0-0-0 116
        110 123 110
        108 114 125
      • exploit folder index.html :
        the reconstruction is very similar to the original
  • Ubuntu 20.04LTS, Nightly v83.0a1 from 2020-10-18:
    • both pages took quite a long time to show the results
      • benchmark folded index.html :
        rgba-0-0-0-0 932
        932 1132 958
        936 938 938
      • exploit folder index.html :
        the reconstruction is very similar to a cube than the original

I do not know what the expected results would be or how I should properly verify this fix.
Lee, Can you help me verify it?

I would recommend the reporter verify.

Flags: needinfo?(lsalzman) → needinfo?(aleksejs)

Thanks for pointing me to the correct nightlies to check!

I have confirmed that I can still observe the difference in timing in Nightly 83.0a1 from 2020-10-01, but cannot observe it in Nightly 83.0a1 from 2020-10-18 (the benchmark shows comparable measurements for the three different colors, and the exploit does not reconstruct the original image, instead showing a random assortment of black/white pixels). So it seems that the fix worked.

Flags: needinfo?(aleksejs)

Setting this bug as Verified based on the comment above. Thank you!

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage] → [reporter-external] [client-bounty-form][post-critsmash-triage]

This grafts cleanly to ESR78. I'm thinking we probably want to uplift this there also?

Flags: needinfo?(lsalzman)

Comment on attachment 9179679 [details]
Bug 1642028 - cherry-pick Skia blitting cleanups. r?jrmuizel

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Timing attack that can be executed from canvas2d in a web page. Just incorporating upstream fixed they already tested and deployed.
  • User impact if declined: Exposure to said timing attack.
  • Fix Landed on Version: 83
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just cherry-picking upstream fix.
  • String or UUID changes made by this patch:
Flags: needinfo?(lsalzman)
Attachment #9179679 - Flags: approval-mozilla-esr78?

Comment on attachment 9179679 [details]
Bug 1642028 - cherry-pick Skia blitting cleanups. r?jrmuizel

skia fix, approved for 78.5esr

Attachment #9179679 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage] → [reporter-external] [client-bounty-form][post-critsmash-triage][adv-main83+]
Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage][adv-main83+] → [reporter-external] [client-bounty-form][post-critsmash-triage][adv-main83+][adv-esr78.5+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.