1272220 - package.json cross-domain-content cors blocked

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[Song Hyo Jin]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160502172042

Steps to reproduce:

its worked addon before
firefox 46 block cors request.

"permissions": {
		"cross-domain-content": [
			"https://translate.googleapis.com/",
			"https://translate.google.com/"
		],
		"unsafe-content-script": true
	}


Actual results:

교차 출처 요청 차단: 동일 출처 정책으로 인해 https://translate.google.com/에 있는 원격 자원을 차단하였습니다. (원인: 'Access-Control-Allow-Origin' CORS 헤더가 없음).

blocked.

cors header not found.


Expected results:

package.json permission allow.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Hey, can you provide the name of the addon and potentially the steps to reproduce the error, than I can take a closer look at this problem. Thanks!

Comment 2

[Song Hyo Jin]

https://addons.mozilla.org/ko/firefox/addon/bridge-translate-app/
this is my app.
thank you.

Comment 3

[Song Hyo Jin]

click translate button, try ajax to google, error CORS block.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Screenshot from 2016-05-13 11:11:40.png

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Screenshot from 2016-05-13 11:11:09.png

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Mhm, currently I can't reproduce the problem. Do I just have to 'Translate' from some langauge to another language. How do you trigger the problem (see my attached two screenshots). Maybe I am doing something wrong.

Comment 7

[Song Hyo Jin]

Attachment: bug-report.png

Comment 8

[Song Hyo Jin]

not webpage. app permission problem. thank you.

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

I can reproduce the problem described within this bug; here are detailed STR:

1) Install https://addons.mozilla.org/ko/firefox/addon/bridge-translate-app/
2) Open App and click [Translate (Ctrl+Alt+Shift+T)]
   Sometimes you have to hit [Text Trans] first and then [Translate] again to trigger the problem
3) The console logs:

> Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://translate.google.com/translate_a/element.js?cb=googleTranslateElementInit. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing).


It seems we have to find the regressing bug for that. Kamil, any chance you can take a look? Reporter claims it used to work before FF46. I am only guessing but potentially it's a regression of Bug 1182571, so maybe that's a good starting point. Thanks!

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

Kamil, please see Comment 9 - thanks!

Comment 11

[Gabor Krizsanits (INACTIVE)]

I think the problem is that we don't create the content sandbox with expanded principal and don't overwrite the XHR ctor. So the content script ends up with a regular content XHR instance. I think it's because the content script is attached with tabs and there we don't send over this info to the child where the sandbox is created: http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/tabs/tab-firefox.js
Luca, do you have some free cycles for this some time?

Comment 12

[Gabor Krizsanits (INACTIVE)]

I've played with this a bit and it seems like CSP of the site has an effect on add-on content scripts.

In my example I tried to use the add-on from the AMO and it has a CSP that blocks using Function. I turned off e10s for this experiment. Since sandbox.js uses content.Function here: https://hg.mozilla.org/mozilla-central/annotate/cad514ad49c1/addon-sdk/source/lib/sdk/content/sandbox.js#l185

with this stack:

 	xul.dll!nsCSPContext::AsyncReportViolation(nsISupports * aBlockedContentSource, nsIURI * aOriginalURI, const nsAString_internal & aViolatedDirective, unsigned int aViolatedPolicyIndex, const nsAString_internal & aObserverSubject, const nsAString_internal & aSourceFile, const nsAString_internal & aScriptSample, unsigned int aLineNum) Line 1122	C++
 	xul.dll!nsCSPContext::LogViolationDetails(unsigned short aViolationType, const nsAString_internal & aSourceFile, const nsAString_internal & aScriptSample, int aLineNum, const nsAString_internal & aNonce, const nsAString_internal & aContent) Line 581	C++
 	xul.dll!nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext * cx) Line 560	C++
>	xul.dll!js::GlobalObject::isRuntimeCodeGenEnabled(JSContext * cx, JS::Handle<js::GlobalObject *> global) Line 549	C++
 	xul.dll!FunctionConstructor(JSContext * cx, unsigned int argc, JS::Value * vp, js::GeneratorKind generatorKind) Line 1760	C++
 	xul.dll!js::Function(JSContext * cx, unsigned int argc, JS::Value * vp) Line 1982	C++
 	xul.dll!js::CallJSNative(JSContext * cx, bool(*)(JSContext *, unsigned int, JS::Value *) native, const JS::CallArgs & args) Line 235	C++
 	xul.dll!js::CallJSNativeConstructor(JSContext * cx, bool(*)(JSContext *, unsigned int, JS::Value *) native, const JS::CallArgs & args) Line 268	C++

we fail. The caller principal is ignored entirely, the function checks the content.Function's subject principal's CSP and banns the call. I think we might run into similar issue with Web Extensions... Kris, have you seen any similar issues? If we never use contentWindow.Function then I guess we shouldn't worry about this but I thought it's worth to keep it in mind...

Comment 13

[Kris Maglione [:kmag]]

I think the problem is that, because the add-on uses "unsafe-content-script", the sandbox is created with the same principal as the content window, and therefore with that principal's CSP. This isn't an issue for WebExtensions, because we always use expanded principals for content scripts there.

Comment 14

[Kris Maglione [:kmag]]

Oh, but as for checking the CSP of the caller rather than the page, that's bug 1267027. The long and short of that is that it's something I'd like to fix, but it will take a lot of work to do correctly.

Comment 15

[Gabor Krizsanits (INACTIVE)]

(In reply to Kris Maglione [:kmag] from comment #14)
> Oh, but as for checking the CSP of the caller rather than the page, that's
> bug 1267027. The long and short of that is that it's something I'd like to
> fix, but it will take a lot of work to do correctly.

Thanks, I just wanted to make sure that this is on the radar.

Comment 16

[Christoph Kerschbaumer [:ckerschb]]

Kris, so is this bug something you are going to fix (if so please reclassify) or is it something I should push into the dom:security backlog?

Comment 17

[Kris Maglione [:kmag]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #16)
> Kris, so is this bug something you are going to fix (if so please
> reclassify) or is it something I should push into the dom:security backlog?

I plan on fixing bug 1267027, which would also fix part of this problem, but I don't have a timeline for that yet.

There are simpler ways to address this, though:

1) Use `evalInSandbox` rather than the Function constructor to construct `unsafeWindow`
2) Create a new principal with the same origin as the content page, but without any CSP applied to it.
3) Remove support for "unsafe-content-script", and just use expanded principals all the time.

I think we should probably do 1 and 3 regardless of any other changes, and 3 would make 2 unnecessary.

Comment 18

[Kamil Jozwiak [:kjozwiak]]

I reproduced the error using the STR from comment #9 using the latest m-c and attempted to get a regression range using mozregression. However, it looks like this has been happening for some time now. I went back as far as fx39.0a1 (2015-03-01) and I could still reproduce the error message via "hit [Text Trans] first and then [Translate]".

Receiving the following error in the browser console:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://translate.google.com/translate_a/element.js?cb=googleTranslateElementInit. (Reason: CORS header 'Access-Control-Allow-Origin' missing).

Chris, let me know if you would like me to go further back, but I don't think this is a regression, or at least a recent regression. The further I go back, the more errors I'm getting in the browser console as the add-ons starts breaking, but the "Cross-Origin Request Blocked" error is still always being displayed under the browser console.

Builds Used: (each one reproduced the error message)
=====================================================

0:24.55 INFO: Running mozilla-central build for 2016-06-06
0:51.46 INFO: application_buildid: 20160606030219
0:51.46 INFO: application_changeset: 0a3b6e2df6567d845f31c000c68dd67816c6153d
0:51.46 INFO: application_version: 49.0a1

0:03.79 INFO: Running mozilla-central build for 2016-01-01
0:18.16 INFO: application_buildid: 20160101030330
0:18.16 INFO: application_changeset: 22f51211915bf7daff076180847a7140d35aa353
0:18.16 INFO: application_version: 46.0a1

0:28.04 INFO: Running mozilla-central build for 2015-10-05
0:51.16 INFO: application_buildid: 20151005030206
0:51.16 INFO: application_changeset: b56aeea0c4701677ffda6417183caa60d2a6a4a7
0:51.16 INFO: application_version: 44.0a1

0:39.25 INFO: Running mozilla-central build for 2015-09-01
1:01.97 INFO: application_buildid: 20150901030226
1:01.97 INFO: application_changeset: cafb1c90f794a73100a8f0afb9fe3301df0f2bde
1:01.97 INFO: application_version: 43.0a1

0:03.42 INFO: Running mozilla-central build for 2015-08-01
0:15.77 INFO: application_buildid: 20150801030211
0:15.77 INFO: application_changeset: aeb85029c3b3
0:15.77 INFO: application_version: 42.0a1

0:23.92 INFO: Running mozilla-central build for 2015-07-05
0:47.25 INFO: application_buildid: 20150705030216
0:47.25 INFO: application_changeset: 136c41fca853
0:47.25 INFO: application_version: 42.0a1

0:21.62 INFO: Running mozilla-central build for 2015-06-01
0:44.69 INFO: application_buildid: 20150601075320
0:44.69 INFO: application_changeset: 666b584fb521
0:44.69 INFO: application_version: 41.0a1

0:03.69 INFO: Running mozilla-central build for 2015-05-01
0:16.46 INFO: application_buildid: 20150501030205
0:16.46 INFO: application_changeset: 7723b15ea695
0:16.46 INFO: application_version: 40.0a1

0:03.43 INFO: Running mozilla-central build for 2015-03-01
0:15.34 INFO: application_buildid: 20150301030224
0:15.34 INFO: application_changeset: eea6188b9b05
0:15.34 INFO: application_version: 39.0a1

Comment 19

[Kris Maglione [:kmag]]

I'm guessing the problem was triggered by Google Translate changing its CORS settings, rather than by a change in Firefox.

Comment 20

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #18)
> Chris, let me know if you would like me to go further back, but I don't
> think this is a regression, or at least a recent regression.

Thanks Kamil, no need to go further back. In fact I think Kris is right in Comment 19, possibly Google Translate changed its CORS settings.

Comment 21

[Luca Greco [:rpl] [:luca] [:lgreco]]

I have nothing to add on the bisect side of this issue, I tried to "hg bisect" it and I have got the same result of Comment 18.

I agree with Kris that it is most likely a change in the CORS settings of the Google Translate website that are triggering this issue.

By taking a look at the Addon SDK tests, both the "cross-domain-content" and "unsafe-content-script" have tests but none of the existent tests are currently checking the behavior of the combination of both the package.json properties applied at the same time in a single add-on, which suggest that they were probably never be intended to work in concert.

Given that the "unsafe-content-script" seems to be a feature introduced to re-enable an historical feature of the content scripts to be able to export object and function to the page without the need of using the cloneInto/exportFunction/createObjectIn helpers, I'm not sure if it would be good thing to open it further (e.g. the "unsafe-content-script" behavior sounds even more dangerous to me if paired with the permission to bypass any CORS restriction, because it makes simpler for the addon to do something wrong that would give to a malicious page new interesting ways to bypass such restrictions, and it make harder for an addon reviewer to notice it).

how about warn the addon developer explicitly when his addon is trying to use these two properties together? 
e.g. using one (or more than one) of this strategies:

- generate a warning at runtime from the Addon SDK itself
- generate a validation error on jpm run/jpm xpi/jpm sign
- generate a validation error on the submission of the add-on on AMO

Comment 22

[Christoph Kerschbaumer [:ckerschb]]

I am going to close this bug as incomplete due to the lack of response.