Closed Bug 1502799 Opened Last year Closed Last year

Implement origin-clean algorithm for ImageBitmap

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 67+ fixed
firefox65 --- fixed

People

(Reporter: baku, Assigned: baku)

References

(Regressed 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html

"An ImageBitmap object's bitmap has an origin-clean flag, which indicates whether the bitmap is tainted by content from a different origin. The flag is initially set to true and may be changed to false by the steps of createImageBitmap()."
Attached patch write_only.patch (obsolete) — Splinter Review
Attachment #9020695 - Flags: review?(aosmond)
Priority: -- → P2
Comment on attachment 9020695 [details] [diff] [review]
write_only.patch

Review of attachment 9020695 [details] [diff] [review]:
-----------------------------------------------------------------

WebGLRenderingContext/WebGL2RenderingContext expose tex(Sub)Image2D and tex(Sub)Image3D which accept an ImageBitmap. I think we need some extra checks around:

https://searchfox.org/mozilla-central/rev/7c848ac7630df5baf1314b0c03e015683599efb9/dom/canvas/WebGLTextureUpload.cpp#223
Attached patch write_only.patchSplinter Review
Attachment #9020695 - Attachment is obsolete: true
Attachment #9020695 - Flags: review?(aosmond)
Attachment #9023032 - Flags: review?(aosmond)
Attachment #9023032 - Flags: review?(aosmond) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5e9d42be7b3
Implement origin-clean algorithm for ImageBitmap, r=aosmond
https://hg.mozilla.org/mozilla-central/rev/a5e9d42be7b3
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Depends on: 1526218
Component: DOM → DOM: Core & HTML
Depends on: CVE-2019-9797
No longer depends on: 1526218
Regressions: 1526218
No longer depends on: CVE-2019-9797
Regressions: CVE-2019-9797

Can you request uplift for esr60? Thanks!

Flags: needinfo?(amarchesini)

Comment on attachment 9023032 [details] [diff] [review]
write_only.patch

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch is the entry point of a good security improvement related to how and when image data should be exposed to cross-origin content.
  • User impact if declined:
  • Fix Landed on Version: 65
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch has been, together with the others, in nightly for a while. Low risk.
  • String or UUID changes made by this patch: none
Flags: needinfo?(amarchesini)
Attachment #9023032 - Flags: approval-mozilla-esr60?
Comment on attachment 9023032 [details] [diff] [review]
write_only.patch

OK for ESR uplift for 60.7.0.
Attachment #9023032 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

Looks like this needs rebasing :\

Flags: needinfo?(amarchesini)

Bug 1484980 - Add selective canvas tainting for content scripts r=bzbarsky
Bug 1526218 - transferFromImageBitmap() should propage the origin-clean state to the canvas element, r=aosmond
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r=aosmond
Bug 1540221 - Setting fillStyle to a pattern of an unclean canvas makes the canvas origin-unclean, r=aosmond

Backed out from esr60 for build bustages on HTMLCanvasElement.cpp.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&resultStatus=testfailed%2Cbusted%2Cexception&revision=ebd350549d83bde2f5e12ec9ea1178d73ee1c25e&selectedJob=246339974

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=246339983&repo=mozilla-esr60&lineNumber=15380

Backout link: https://hg.mozilla.org/releases/mozilla-esr60/rev/9d3b9a48a91710e8bf4eb36c5181e391c7c14abe

[task 2019-05-14T08:48:26.175Z] 08:48:26 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/html'
[task 2019-05-14T08:48:26.179Z] 08:48:26 INFO - /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_dom_html0.i_o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/html -I/builds/worker/workspace/build/src/obj-firefox/dom/html -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/caps -I/builds/worker/workspace/build/src/docshell/base -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/canvas -I/builds/worker/workspace/build/src/dom/html/input -I/builds/worker/workspace/build/src/dom/media -I/builds/worker/workspace/build/src/dom/security -I/builds/worker/workspace/build/src/dom/xbl -I/builds/worker/workspace/build/src/dom/xul -I/builds/worker/workspace/build/src/editor/txmgr -I/builds/worker/workspace/build/src/image -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/tables -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/netwerk/base -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fomit-frame-pointer -Werror -Wno-error=shadow -fprofile-generate -MD -MP -MF .deps/Unified_cpp_dom_html0.i_o.pp /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp
[task 2019-05-14T08:48:26.180Z] 08:48:26 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp:65:0:
[task 2019-05-14T08:48:26.181Z] 08:48:26 INFO - /builds/worker/workspace/build/src/dom/html/HTMLCanvasElement.cpp:940:6: error: prototype for 'bool mozilla::dom::HTMLCanvasElement::IsWriteOnly()' does not match any in class 'mozilla::dom::HTMLCanvasElement'
[task 2019-05-14T08:48:26.182Z] 08:48:26 INFO - bool HTMLCanvasElement::IsWriteOnly() { return mWriteOnly; }
[task 2019-05-14T08:48:26.183Z] 08:48:26 INFO - ^~~~~~~~~~~~~~~~~
[task 2019-05-14T08:48:26.183Z] 08:48:26 INFO - In file included from /builds/worker/workspace/build/src/dom/html/HTMLCanvasElement.cpp:7:0,
[task 2019-05-14T08:48:26.184Z] 08:48:26 INFO - from /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp:65:
[task 2019-05-14T08:48:26.185Z] 08:48:26 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/HTMLCanvasElement.h:204:8: error: candidate is: bool mozilla::dom::HTMLCanvasElement::IsWriteOnly() const
[task 2019-05-14T08:48:26.185Z] 08:48:26 INFO - bool IsWriteOnly() const;
[task 2019-05-14T08:48:26.186Z] 08:48:26 INFO - ^~~~~~~~~~~
[task 2019-05-14T08:48:26.187Z] 08:48:26 INFO - /builds/worker/workspace/build/src/config/rules.mk:1054: recipe for target 'Unified_cpp_dom_html0.i_o' failed
[task 2019-05-14T08:48:26.188Z] 08:48:26 INFO - make[5]: *** [Unified_cpp_dom_html0.i_o] Error 1
[task 2019-05-14T08:48:26.188Z] 08:48:26 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/html'
[task 2019-05-14T08:48:26.189Z] 08:48:26 INFO - /builds/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'dom/html/target' failed
[task 2019-05-14T08:48:26.190Z] 08:48:26 INFO - make[4]: *** [dom/html/target] Error 2
[task 2019-05-14T08:48:26.190Z] 08:48:26 INFO - make[4]: *** Waiting for unfinished jobs....

New patch submitted

Flags: needinfo?(amarchesini)
Attachment #9064518 - Attachment is obsolete: true

Sent a patch to enabled them again. My mistake.

Flags: needinfo?(amarchesini)

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

Why do you leave docstring define_css_keyword_enum which is part of bug 1519729? Either backport all fixes for later Rust versions properly[1] or backout all unrelated changes.

[1] For example:
https://github.com/freebsd/freebsd-ports/blob/1e2726401874/www/firefox-esr/files/patch-bug1519629
https://github.com/freebsd/freebsd-ports/blob/1e2726401874/www/firefox-esr/files/patch-bug1519729

This patch is meant to land in esr-60. It's not going to be in m-i/m-c. I just want to remove the rust changes in my previous patch. Bug 1519729 landed in 66 and it's not going to be ported to esr-60.

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: In the previous patch I removed and added comments to rust code. Those changes were meant to be just for testing. Would be nice if we can remove them.
  • User impact if declined: none
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch introduces what I removed/added in the previous code and it's completely unrelated to imagebitmap
  • String or UUID changes made by this patch: none
Attachment #9065302 - Flags: approval-mozilla-esr60?

We're already built 60.7.0, But, if we end up having a 60.7.1 dot release, we can take this as a ridealong. I'll leave the uplift request open for now.

Attachment #9065302 - Attachment is obsolete: true
Attachment #9065302 - Flags: approval-mozilla-esr60?

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: I didn't want to cancel this esr-60 request.
  • User impact if declined:
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String or UUID changes made by this patch:
Attachment #9065302 - Flags: approval-mozilla-esr60?
Attachment #9065302 - Attachment is obsolete: false

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

Fixes a mistake in the rebased ESR60 patch. Approved for 60.8esr.

Attachment #9065302 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Regressions: 1558797
You need to log in before you can comment on or make changes to this bug.