Closed Bug 1004260 Opened 8 years ago Closed 8 years ago

Firefox 29 broke clipboard access; restore the CAPS allowclipboard policy until the Clipboard API allows click-to-copy

Categories

(Core :: Security: CAPS, defect)

29 Branch
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: developerjosh, Unassigned)

References

Details

(Keywords: dev-doc-complete, regression, site-compat)

Attachments

(1 file, 4 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140421221237

Steps to reproduce:

You have a webpage here that describes how to enable Firefox clipboard access:

http://www-archive.mozilla.org/editor/midasdemo/

But changing the security preferences, as described here:

https://developer.mozilla.org/en-US/docs/Midas/Security_preferences

no longer works.


Actual results:

Security Exception results in the console


Expected results:

The program should have allowed clipboard access
Yes, this is intended.
https://developer.mozilla.org/en-US/Firefox/Releases/29/Site_Compatibility#Security
Blocks: 913734
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Component: Untriaged → Editor
Product: Firefox → Core
Resolution: --- → WONTFIX
Bug 995943 Comment 136 and other reports say CKeditor's cut/copy buttons are also broken.

https://support.mozilla.org/en-US/questions/1001086

I think this should also be restored for backward compatibility until "semi-trusted events" of the Clipboard API are implemented. Hallvord, what's the current plan?

http://lists.w3.org/Archives/Public/public-webapps/2014JanMar/0087.html
https://www.w3.org/Bugs/Public/show_bug.cgi?id=23235
Status: RESOLVED → REOPENED
Ever confirmed: true
Flags: needinfo?(hsteen)
Resolution: WONTFIX → ---
Status: REOPENED → NEW
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Update 29 Broke Clipboard Access → Firefox 29 broke clipboard access; restore the CAPS allowclipboard policy until the Clipboard API allows click-to-copy
Component: Editor → Security: CAPS
If the allowclipboard policy is not restored, people and companies relying on that functionality have to develop a custom extension to do the same thing, or switch to a Flash-based solution used on GitHub, Bitly, etc. especially for click-to-copy URL buttons. The latter might be easy to implement but what Mozilla don't want to see. In my understanding the standard Clipboard API works well once the click-to-copy support is implemented.

The allowclipboard policy might be used for corporate internal apps. Note that Firefox 31, the next ESR, is already Aurora.
Flags: needinfo?(bobbyholley)
Some companies disallow end users to install Flash nor extensions. In that case, they probably won't have any workaround. Keyboard shortcuts may still work though, of course.

I'm also seeing some related topics on forums posted by end users, say cut/copy/paste buttons offered by rich editors are no longer working.

http://forums.mozillazine.org/viewtopic.php?f=9&t=2826389
https://support.mozilla.org/en-US/questions/984029
https://support.mozilla.org/en-US/questions/1000877
https://support.mozilla.org/en-US/questions/1001146
Keywords: regression
Regression and affect plenty of users. It would be nice to have a fix at least for 31 for the next ESR.
Regarding the plan, I realised we don't have a bug on implementing click-to-copy so I've now reported bug 1012662
Flags: needinfo?(hsteen)
See Also: → 1012662
I think this is a much bigger issue then we realize because CKEditor uses it which is one of the most widely based web editors.
One of the most popular blog service called Ameba Blog, like Blogger, also offers copy/paste buttons and the help article recommends Firefox users to add the CAPS prefs if they want to use the buttons. Not surprisingly, they have been confused after upgrading to Firefox 29.

http://helps.ameba.jp/faq/blog/article/post_77.html
I meant: popular *Japanese* blog service
This isn't a 'regression' per se, but an intentionally-removed anti-web feature. The mechanism being used here is Firefox-specific and was never going to be standardized or implemented in other UAs.

IIUC, the current state-of-the art for clipboard access is to use a small flash plugin. This is unfortunate, but I honestly thing it's preferable to using CAPS, because it's somewhat portable between UAs.

Restoring support for this in a modern Gecko requires a significant amount of engineering effort, and I don't want to do it unless we really need to (like we did with bug 995943). So far, I'm not convinced that that's the case here.
Flags: needinfo?(bobbyholley)
bholley: Is it less work to implement click-to-copy?

Note that per the spec, semi-trusted events will only allow "copy" and "cut" actions - not "paste". The spec still assumes that the UA will have some (likely site-specific) way to enable script-triggered paste action.

(The logic here is that a site finding a way to abuse cut/copy is generally merely a nuisance, overwriting clipboard data - but a site finding a way to abuse paste is a privacy issue and serious threat - monitoring user data and behaviour it shouldn't. Hence different security policies for copy/cut and paste.)

If "the CAPS allowclipboard policy" was the only way to enable document.execCommand('paste'), we should keep it enabled until we've implemented a functional replacement. I don't know the details - if I need to report another bug on "make pref to control script-triggered paste" I hope somebody who knows the implementation tells me to do so!
Flags: needinfo?(bobbyholley)
(In reply to Hallvord R. M. Steen from comment #12)
> bholley: Is it less work to implement click-to-copy?

The work relates to having site-specific security preferences in general.
 
> Note that per the spec, semi-trusted events will only allow "copy" and "cut"
> actions - not "paste". The spec still assumes that the UA will have some
> (likely site-specific) way to enable script-triggered paste action.

Implicitly relying on non-standard features that require users to configure their UA in proprietary ways seems pretty suboptimal IMO.

> (The logic here is that a site finding a way to abuse cut/copy is generally
> merely a nuisance, overwriting clipboard data - but a site finding a way to
> abuse paste is a privacy issue and serious threat - monitoring user data and
> behaviour it shouldn't. Hence different security policies for copy/cut and
> paste.)

Sure. But I think that any security policy here should not rely on users going to about:config.

> If "the CAPS allowclipboard policy" was the only way to enable
> document.execCommand('paste'), we should keep it enabled until we've
> implemented a functional replacement. I don't know the details - if I need
> to report another bug on "make pref to control script-triggered paste" I
> hope somebody who knows the implementation tells me to do so!

Do other UAs have site-specific preferences that let web pages access the clipboard? AFAICT, the only solution in Chrome is to use an extension, an approach that works equally-well in Firefox 29.
Flags: needinfo?(bobbyholley)
Unfortunately, this is one of those areas where web apps need functionality, and the browser and standards have been incredibly slow. IE is actually the leader in this error.

The about:config stuff is cumbersome, but it at least gives a solution for doing real rich text editing in the browser.

Would it be possible to use any of the code from bug 995943 for this?

As a side note, when you removed caps, did you remove the code in about:config that hid all the caps preferences? I always hated that they were hidden.
(In reply to Mike Kaply (:mkaply) from comment #14)
> The about:config stuff is cumbersome, but it at least gives a solution for
> doing real rich text editing in the browser.

Any more so than a flash applet or an addon? Is the issue that there's also an IE-specific way to allow document.executeCommand, and doing the about:config dance lets IE-proprietary code kinda-sorta work in Firefox too?

> Would it be possible to use any of the code from bug 995943 for this?

To some extent, yes, but it would be more special code implementing an approach that, even out of many imperfect solutions, seems like one of the worst. So I don't want to support it unless we need to.

> As a side note, when you removed caps, did you remove the code in
> about:config that hid all the caps preferences? I always hated that they
> were hidden.

I don't remember touching anything like that.
> > bholley: Is it less work to implement click-to-copy?
> 
> The work relates to having site-specific security preferences in general.

Just to be very precise (sorry if I nitpick): implementing "click-to-copy" won't have to rely on site-specific prefs - it's applied to all sites. The general idea is that if Firefox would allow a given script to trigger a popup, it will allow the same script to execute a copy or cut command.

> > The spec still assumes that the UA will have some
> > (likely site-specific) way to enable script-triggered paste action.
> 
> Implicitly relying on non-standard features that require users to configure
> their UA in proprietary ways seems pretty suboptimal IMO.

Quite possibly "suboptimal" - but there are only two scenarios if we want native "paste" in the web platform:
* allow all pages/scripts to do it
* trust some sites more than others (and accept that the spec can't dictate the UI browsers will have to invent to indicate site trust, because W3C specs don't tend to make UI decisions)

> Sure. But I think that any security policy here should not rely on users
> going to about:config.

Nobody said anything about about:config :). Personally, I think the best solution is one of those yellow bar widgets saying for example "www.mozilla.org wants to access your clipboard data. Allow?". The exact UI is up to UI people though - 

> Do other UAs have site-specific preferences that let web pages access the
> clipboard?

Some of them certainly do!

Presto-Opera had this. [F12]->Site-specific preferences->Scripting->Allow access to clipboard.

More relevant for real-world usage: IE has it, and has had it for quite a while I believe. If a script calls document.execCommand('paste'), it pops up a modal dialog asking if you want to give this site access to the clipboard. Given that they basically invented document.execCommand() and it's been there since IE 6 or earlier, I think this an old UI/pref feature.
(In reply to Bobby Holley (:bholley) from comment #15)
> (In reply to Mike Kaply (:mkaply) from comment #14)
> > As a side note, when you removed caps, did you remove the code in
> > about:config that hid all the caps preferences? I always hated that they
> > were hidden.
> 
> I don't remember touching anything like that.

Someone else did it in bug 284673
(In reply to Hallvord R. M. Steen from comment #16)
> Nobody said anything about about:config :).

That's what the recently-removed mechanism involves, and why I'm opposed to re-instating it.

> Personally, I think the best
> solution is one of those yellow bar widgets saying for example
> "www.mozilla.org wants to access your clipboard data. Allow?". The exact UI
> is up to UI people though - 

I absolutely agree!

> More relevant for real-world usage: IE has it, and has had it for quite a
> while I believe. If a script calls document.execCommand('paste'), it pops up
> a modal dialog asking if you want to give this site access to the clipboard.
> Given that they basically invented document.execCommand() and it's been
> there since IE 6 or earlier, I think this an old UI/pref feature.

That is useful to know - thanks.
I do agree to remove CAPS, but it seems the allowclipboard policy has been widely used especially in internal apps. Since enterprise users first knew the removal of CAPS when :mkaply posted to the enterprise list on May 5th and most of them are using ESR, it's not surprise that we haven't got a bunch of bug reports and complaints. But this could potentially affect thousands of users when Firefox ESR 31 is out.

If it's difficult to add back the allowclipboard policy support now, we could implement the standard click-to-copy support in Firefox 32 as soon as possible and uplift the code to Firefox 31.
(In reply to Bobby Holley (:bholley) from comment #18)
> (In reply to Hallvord R. M. Steen from comment #16)
> > Nobody said anything about about:config :).
> 
> That's what the recently-removed mechanism involves, and why I'm opposed to
> re-instating it.

It doesn't necessarily involve about:config, it simply involves prefs. Those prefs could be set via a restartless add-on or they could be preset as they are in an enterprise.

In a perfect world, this would be just another site specific permission on about:permissions but I'm not sure how extensible that interface is. Maybe that's the right thing to do here?

Do you believe the document.execCommand part could be done with an extension? Or when you say extension, do you mean inventing something completely new?

The core problem here is there are lots of apps out in the wild that use this and this was a benefit of Firefox - you could use the clipboard in your web app.

On a side note, I can't believe that no one has solved this problem. Everyone wants to builds things that work like desktop apps in the browser, but if you don't have clipboard, it's a pretty significant hole...
> another site specific permission on about:permissions

I didn't know about about:permissions, but it looks great - bug 1013165 reported on adding a new "Allow clipboard access" entry there.
We've got literally thousands of users affected at my organization (CKeditor is embedded in our internal app). Some people just can't get going with keyboard shortcuts. We struggle to keep application as lightweight and proprietary components-free as possible, so that state-o-the-art flash thingy won't solve it for us; deploying a hypothetical add-on will have deployment, sustainability, or security implications for some people here.
Our bad we didn't deploy ESR you may argue, but while we're at it now, not having any foreseeable sort of solution in the next ESR leaves us kinda.. abandoned, to keep the post polite :)
l2jdrlecter, how do you deploy the non-default preference values right now?
A mangled user.js is whether distributed or already included in disk images. We could get going with a security configuration option, a one time UI confirm dialog, whatever about:config/permission/* setting, as far as we get that capability restored.
Deploying an add-on via those disk images wouldn't work?
(In reply to Boris Zbarsky [:bz] from comment #25)
> Deploying an add-on via those disk images wouldn't work?

Do you believe an add-on can restore the exact behavior (execCommand cut/copy/paste)?
If it can't, then we should probably either restore the core bits or ship an actual API for this.
An addon could watch for DOM windows of whitelisted sites, and shim in a replacement for HTMLDocument.prototype.execCommand that re-routed clipboard commands to privileged code.
Will an addon's injected scripts run by default inside IFRAMEs (including about:blank)? That's where you find the execCommand() you need to interfere with. Will you be able to maintain the faked execCommand when the editors do their weird stuff in and to the document? Editors do the oddest things, including using document.write() to replace contents of IFRAMEs they've already loaded about:blank into. Also watch out for those unexpected documents coming from <iframe src="javascript:'some str possibly with script doing doc.write'">. Making sure you overwrite the execCommand in the right document for all versions of the editor script is a real timing conundrum!

(I've been there, done that for Opera's browser.js, not clipboard stuff but fiddling with other rich text editing problems with pretty extensive attempts at patching both CKEditor and TinyMCE - this stuff can get ugly fast..)

Fixing bug 1012662 (click-to-copy, basically allow execCommand('copy') and execCommand('cut') from click and some other events) and 1013165 (site pref to allow execCommand('paste') in about:permissions) would solve all the issues. To me, neither of those bugs seem hard to fix..
See Also: → 1013165
(In reply to Hallvord R. M. Steen from comment #29)
> Will an addon's injected scripts run by default inside IFRAMEs (including
> about:blank)? That's where you find the execCommand() you need to interfere
> with. Will you be able to maintain the faked execCommand when the editors do
> their weird stuff in and to the document? Editors do the oddest things,
> including using document.write() to replace contents of IFRAMEs they've
> already loaded about:blank into. Also watch out for those unexpected
> documents coming from <iframe src="javascript:'some str possibly with script
> doing doc.write'">. Making sure you overwrite the execCommand in the right
> document for all versions of the editor script is a real timing conundrum!
> 
> (I've been there, done that for Opera's browser.js, not clipboard stuff but
> fiddling with other rich text editing problems with pretty extensive
> attempts at patching both CKEditor and TinyMCE - this stuff can get ugly
> fast..)

Yeah, I agree that it could get pretty tricky.

> Fixing bug 1012662 (click-to-copy, basically allow execCommand('copy') and
> execCommand('cut') from click and some other events) and 1013165 (site pref
> to allow execCommand('paste') in about:permissions) would solve all the
> issues. To me, neither of those bugs seem hard to fix..

Yeah, those both seem like the right way to go. Just need somebody to do it.
So I guess we can close this bug as WONTFIX, then Bug 1012662 and Bug 1013165 for Firefox 31 instead.
grrr, ... then *track and implement* Bug 1012662 and Bug 1013165 for Firefox 31 (next ESR)
(In reply to Kohei Yoshino [:kohei] from comment #32)
> grrr, ... then *track and implement* Bug 1012662 and Bug 1013165 for Firefox
> 31 (next ESR)

That sounds good, but we will need an owner.
Removing FF30 tracking and marking wontfix.  Will leave the tracking for 31 on here for now just so this remains on our radar until comment 32 is put into action.
Attached file Extension to bring back allowclipboard (obsolete) —
Here's an extension to bring back the function.

Can someone else take a look at my bootstrap.js and see what they think?

Once someone else has verified the code, I'm going to convert it to non restartless and integrate it into the CCK2.
Attached file The actual code (obsolete) —
Here's the actual code so you don't have to unpack the XPI.
Attached file The real actual code (obsolete) —
It would help if I wrote the policy code correctly.

Here's the correct code.
Attachment #8428499 - Attachment is obsolete: true
Attachment #8428500 - Attachment is obsolete: true
This looks great! Thanks for doing that Mike.

For security, I'd suggest only overriding the setter in the case that the given domain is an allowed site. I'd also suggest using Cu.exportFunction [1] to make this more future-proof.

[1] https://developer.mozilla.org/en-US/docs/Components.utils.exportFunction
> I'd also suggest using Cu.exportFunction [1] to make this more future-proof.

I can't do that with the getter, though, can I?
(In reply to Mike Kaply (:mkaply) from comment #39)
> > I'd also suggest using Cu.exportFunction [1] to make this more future-proof.
> 
> I can't do that with the getter, though, can I?

Why not? Just don't use defineAs, and use the return value from exportFunction.
The "this" pointer in exportFunction is the sandbox, and I have no access to document or any of the other variables I need.

Is there any way to access the scope that I'm placed in to?
Attached file Address bholley's comments (obsolete) —
Attachment #8429552 - Flags: review?(bobbyholley)
Comment on attachment 8429552 [details]
Address bholley's comments

(In reply to Mike Kaply (:mkaply) from comment #41)
> Is there any way to access the scope that I'm placed in to?

Using a closure. Looks like this is what you did.

Nit - It would be easier to review this as a patch, because as it stands I can't use splinter-review.

> doc.originalDesignModeGetter = doc.__lookupSetter__("designMode");

Shouldn't this be called originalDesignModeSetter?

> var doc = subject.wrappedJSObject.document;

This waives Xrays too early. In particular, this means that your expandos (originalDesignModeGetter, allowCutCopy, allowPaste) will be content visible, which probably isn't what you want.

I think you should get rid of the wrappedJSObject at the top, and make the bottom look like this:

doc.originalDesignModeGetter = doc.wrappedJSObject.__lookupSetter__("designMode");
doc.wrappedJSObject.__defineSetter__("designMode", Components.utils.exportFunction(myDesignModeSetter(doc), doc));

And possibly also waive xrays before computing doc.execCommand if you think that might be monkey-patched by someone else as well.
Attachment #8429552 - Flags: review?(bobbyholley) → feedback+
Attachment #8428505 - Attachment is obsolete: true
Attachment #8429552 - Attachment is obsolete: true
Attachment #8429598 - Flags: review?(bobbyholley)
Comment on attachment 8429598 [details] [diff] [review]
Use wrappedJSObject properly

This doesn't solve the nit. Click 'review' on your patch to see what I mean.
Attachment #8429598 - Flags: review?(bobbyholley)
> This doesn't solve the nit. Click 'review' on your patch to see what I mean.

I still have no idea what you are asking.

I've never used splinter reviews.

What do I do besides marking the patch for review?
The file your are attaching is not a patch - it's just a javascript file, which breaks the reviewer tooling.
I'll just review it as it is.
Comment on attachment 8429598 [details] [diff] [review]
Use wrappedJSObject properly

>function myExecCommand(doc) {
>  return function(aCommandName, aShowDefaultUI, aValueArgument) {
>    switch (aCommandName) {
>      case "cut":
>      case "copy":
>	if (doc.allowCutCopy) {
>	  var win = Services.wm.getMostRecentWindow("navigator:browser");
>	  win.goDoCommand("cmd_" + aCommandName);
>	  return;
>	}
>	break;
>      case "paste":
>	if (doc.allowPaste) {
>	  var win = Services.wm.getMostRecentWindow("navigator:browser");
>	  win.goDoCommand("cmd_" + aCommandName);
>	  return;
>	}
>	break;
>    }
>    doc.originalExecCommand(aCommandName, aShowDefaultUI, aValueArgument);

Hm, don't you need to .call this with |doc| as |this|, otherwise you'll get the wrong this-binding?

I would suggest using Cu.waiveXrays(doc).bar instead of doc.wrappedJSObject.bar, so that this handles any odd cases where the object isn't an XrayWrapper (and thus doesn't have a .wrappedJSObject property).

Looks good otherwise.
Attachment #8429598 - Flags: review+
> The file your are attaching is not a patch - it's just a javascript file, which breaks the reviewer tooling.

By patch, you mean diff. Got it. Sorrry. 

> Hm, don't you need to .call this with |doc| as |this|, otherwise you'll get the wrong this-binding?

No, because the originalExecCommand is attached to the doc, so it will run in the same context.

> I would suggest using Cu.waiveXrays(doc).bar

I can't find any documentation on waiveXRays...
(In reply to Mike Kaply (:mkaply) from comment #50)
> > The file your are attaching is not a patch - it's just a javascript file, which breaks the reviewer tooling.
> 
> By patch, you mean diff. Got it. Sorrry. 
> 
> > Hm, don't you need to .call this with |doc| as |this|, otherwise you'll get the wrong this-binding?
> 
> No, because the originalExecCommand is attached to the doc, so it will run
> in the same context.

I don't follow this.

If you do |let func = doc.foo; func();| You'll end up invoking func with the wrong this-binding, so you need to do func.call(doc).
 
> > I would suggest using Cu.waiveXrays(doc).bar
> 
> I can't find any documentation on waiveXRays...

http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/idl/xpccomponents.idl#509

It's equivalent to foo.wrappedJSObject, except it doesn't break your code when |foo| happens to not be an XrayWrapper.
> If you do |let func = doc.foo; func();| You'll end up invoking func with the wrong this-binding, so you need to do func.call(doc).

That's not what I'm doing.

I'm doing:

doc.func = func();

And calling doc.func();

So the this pointer is correct.
Oh! I see. Yes, that should be fine then. :-)
> Oh! I see. Yes, that should be fine then. :-)

That's the reason I tend to attach things to the original objects. So I don't have to worry about call/apply.
The allowclipboard policy support itself won't be back, so marking WONTFIX here. Let's track Bug 1012662 and Bug 1013165 instead.
Status: NEW → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.