1021972 - csp attribute in manifest doesnt actually apply a CSP

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

While testing CSP with the new parser on flame device (ie with security.csp.speccompliant set to true), i noticted that the csp attribute doesnt actually tighten the CSP as it is supposed to. 

STR:
1. Install the app from https://github.com/pauljt/csptest using app manager
2. use the app, observer the CSP blocks the inline styles
3. change the app type to privileged or web
4. try the test cases again

Expected:
The forbidden styles should still be blocked, since the mainfest applies a csp.

Actual:
Inline styles are not blocked.

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

This will likely block 2.1 I expect there be a requirement to be able to apply specific CSP to apps. We will also need to be very careful how we solve this, since there are already a number of apps in the appstore which use a CSP. 

cr, how many apps are using this csp attribute (out of how many packaged applications)

Comment 2

[Christiane Ruetten [INACTIVE]]

I am counting 29 unique app IDs with CSP set, 6 of them being privileged apps, out of 400 privileged app IDs, and out of 2443 app IDs in total in my s3 mirror.

Mind that I am counting per app ID and not multiple versions here, but I don't differentiate by publication status.

The current list is:

420638/public/transgo-immh-1.0/manifest.webapp	script-src 'self' 'unsafe-inline'; 
423632/public/iroller-3.2.0/manifest.webapp	allow 'self'; script-src 'self' ; style-src 'self';
429092/pending/whiteout-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
432494/public/splat-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.3/manifest.webapp	default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.4/manifest.webapp	default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.5/manifest.webapp	default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
440302/pending/nomorpenting-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
440750/public/loyer-impaye-1.1/manifest.webapp	default-src *;  default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline'
446280/public/calculator-13-1.0/manifest.webapp	default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
446280/public/calculator-13-1.0.1/manifest.webapp	default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
449898/public/contacts-backup-0.2/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-0.9/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.01/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.1/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.11/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
451154/public/local-fresh-1.1/manifest.webapp	default-src *; script-src 'self' http://*.googleapis.com; object-src 'none'; style-src 'self' 'unsafe-inline'; connect-src *
453212/public/gisthub-1.0.1/manifest.webapp	default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
456318/pending/orsys-1.0/manifest.webapp	default-src *;  default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.0/manifest.webapp	default-src *;  default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.2/manifest.webapp	default-src *;  default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.3/manifest.webapp	default-src *;  default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
457642/public/ripples-1-1.0/manifest.webapp	default-src 'self'; img-src 'self'; script-src 'self';
457909/public/foxmote-0.3/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.4/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.6/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.8/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self'
458778/public/sr100simon-1.1/manifest.webapp	default-src 'self'; img-src 'self'; script-src 'self';
461644/public/katagana-1.1/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
461644/public/katagana-1.2/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
462457/pending/mobile-app-1.0/manifest.webapp	default-src *; script-src 'self' http://*.googleapis.com; object-src 'none'; style-src 'self' 'unsafe-inline'; connect-src *
464126/public/black-star-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
468310/public/mahjong-helper-2.40.2/manifest.webapp	default-src 'self' mahjonghelper.com
471904/pending/guiato-0.1/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self'
474628/public/lottoszam-generator-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
476422/pending/debt-list-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
478383/public/magaly-1-2.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
478383/public/magaly-1-3.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483047/pending/flamita-3.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483337/pending/asmaul-husna-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483419/public/asmaul-husna-1-1.0/manifest.webapp	default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
484230/public/arrow-1.2.0/manifest.webapp	allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Is this limited to inline style blocking?  

Is security.csp.speccompliant set to true?

Paul: can you try the STR with an image load that would violate the CSP in manifest but not the app default CSP?

Comment 4

[Jason Smith [:jsmith]]

I'm adding qawanted to branch checks.

Is there a specific visual we can check for here to know if the expected or actual results are hit?

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

re: Jason - I tried to add explanatory comments to the app. But for the moment these explanations aren't correct since we allow inline styles for certified apps. In order to test as intended you need to set the certified CSP back to blocking script: 

pref("security.apps.certified.CSP.default", "default-src *; script-src 'self'; object-src 'none'; style-src 'self'");


re: Sid: added a simple image test the test app, and test with multiple CSP permutations.

STR:
1. install the app from https://github.com/pauljt/csptest
2. open the app, scroll to the bottom and press the "external image" link
3. If you see a picture of gunter the penguin, the external image is loading.

The csp I tested with is:
"csp": "image-src 'self'; style-src 'self';"

But I also tried:

"csp": "image-src 'none'";

And I tried with both privileged and certified app types - the image was never blocked.

Comment 6

[Paul Theriault [:pauljt] (no longer reading bugmail)]

PS I am using a nightly build, so the speccompliant is set to true. I checked this by manually toggling the pref in prefs.js.

Comment 7

[Sid Stamm [:geekboy or :sstamm]]

Paul, any chance you can try removing the fast path and rebuild gecko to see if that's causing the issue?  I don't have a B2G build spinned up so it might be faster for you to test this out than for me.

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.cpp#111

Comment 8

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #7)
> Paul, any chance you can try removing the fast path and rebuild gecko to see
> if that's causing the issue?  I don't have a B2G build spinned up so it
> might be faster for you to test this out than for me.
> 
> http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.
> cpp#111

Sure thing, building at the moment. But I see the same result for both certified and privileged apps and fast path only seems to apply to certified iiuc. So not sure this is it.

Comment 9

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Actually, from a post Andrew made on the marketplace list, I think this might be a side-effect of push apps rather going through the proper install process. Can you confirm this is the case Andrew, Fabrice?

Comment 10

[[:fabrice] Fabrice Desré]

(In reply to Paul Theriault [:pauljt] from comment #9)
> Actually, from a post Andrew made on the marketplace list, I think this
> might be a side-effect of push apps rather going through the proper install
> process. Can you confirm this is the case Andrew, Fabrice?

very likely (and a valid bug). Can you paste your /data/local/webapps/webapps.json file after you pushed the app?

Comment 11

[Andrew Williamson [:eviljeff]]

(In reply to Paul Theriault [:pauljt] from comment #9)
> Actually, from a post Andrew made on the marketplace list, I think this
> might be a side-effect of push apps rather going through the proper install
> process. Can you confirm this is the case Andrew, Fabrice?

if its been pushed via app-manager then the CSP rules get bypassed, yeah.  Or at least have done up to now - I don't know about effect of the parser mentioned in comment #0.  The zip would have to be installed via the normal js functions.  If its privileged then first uploaded to somewhere valid (e.g. marketplace-dev.allizom.org) and installed from there.  I can give you reviewer access to -dev to do your own approvals if that helps.

Comment 12

[Jason Smith [:jsmith]]

I'm clearing qawanted because the bug as filed here is targeting an app manager workflow, which is only dev driven, not user driven. We need proof of this happening from installation from the web to consider blocking on this btw.

I'm also doubtful this is a regression. We've had many bugs historically happen where we didn't ever bother implementing the workflow in the app manager workflow.

I'm adding needinfo on Alexandre to check to see if my theory above is correct that this is an issue only seen if the app is pushed directly to the device, not by installation off the web.

Comment 13

[Alexandre Poirot [:ochameau]]

Not a regression from app manager side. I don't think we ever supported csp defined in manifest.

If somone confirms that install workflow from marketplace works as expected, do not hesitate to move this bug to Firefox::Devtools Web IDE component.

Comment 14

[Paul Theriault [:pauljt] (no longer reading bugmail)]

So I installed the "arrow" app from the marketplace. It has a csp, which you can verify in the app manager console:

> app.manifest.csp
< "allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';"

So it has a csp set that should block inline styles and scripts.

But then in app manager, I can do: 

window.eval('1+1');

Maybe app manager has super powers to bypass CSP restrictions in apps? Seems unlikely though. I'll try with a self installed app.

Comment 15

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Ok testing locally on 2.0 simulator, with the following csp attribute for a "web" type app works.

"csp":"img-src 'self'; script-src 'self'; style-src 'self';"

If I then type "eval(1)" into app manager, I get a csp error.

So I guess next step is checking with an app that I wrote myself installed from staging marketplace.But I am out of time for this week, so I will have to try this next week.

Comment 16

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Fabrice Desré [:fabrice] from comment #10)
> (In reply to Paul Theriault [:pauljt] from comment #9)
> > Actually, from a post Andrew made on the marketplace list, I think this
> > might be a side-effect of push apps rather going through the proper install
> > process. Can you confirm this is the case Andrew, Fabrice?
> 
> very likely (and a valid bug). Can you paste your
> /data/local/webapps/webapps.json file after you pushed the app?

FWIW, here what the entry for my earlier attempt (sideloaded app via app manager):

 "0f848c91-0e82-8f49-982b-6b53a38ea5dc": {
    "origin": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc",
    "installOrigin": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc",
    "manifestURL": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc/manifest.webapp",
    "appStatus": 3,
    "receipts": [],
    "installTime": 1404778234593,
    "installState": "installed",
    "removable": true,
    "id": "0f848c91-0e82-8f49-982b-6b53a38ea5dc",
    "basePath": "/data/local/webapps",
    "localId": 1068,
    "name": "CSP Test",
    "redirects": null
  }


As expected, csp parameter is not set.

Comment 17

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Ok this is wierd. For the arrow app i see this in webapps.json:

...
"{6cc39c39-df1e-43e4-b372-bf0aa90ec65c}": {
    "name": "Arrow",
    "csp": "",
    "installOrigin": "https://marketplace.firefox.com",
...

But if you look at the manifest for Arrow, 

root@flame:/data/local/webapps/{6cc39c39-df1e-43e4-b372-bf0aa90ec65c} # cat manifest.webapp
{"name":"Arrow","description":"The Web Synthesizer","version":"1.2.0","csp":"allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';","launch_path":"/index.html","orientation":["landscape"],"fullscreen":"true","icons":{"16":"/img/webapp/icon16.png","30":"/img/webapp/icon30.png","32":"/img/webapp/icon32.png","48":"/img/webapp/icon48.png","60":"/img/webapp/icon60.png","64":"/img/webapp/icon64.png","90":"/img/webapp/icon90.png","120":"/img/webapp/icon120.png","128":"/img/webapp/icon128.png","256":"/img/webapp/icon256.png","512":"/img/webapp/icon512.png"},"developer":{"name":"Daniele Veneroni","url":"http://venerons.github.io/"},"default_locale":"en","locales":{"it":{"description":"The Web Synthesizer"}}}

Is that expected?

Comment 18

[[:fabrice] Fabrice Desré]

I can't imagine that to be hard to fix. I'll take a look next week if no-one steps up.

Comment 19

[Sheila Mooney]

This has been nominated as a 2.1 blocker for a while. Who is the right person to triage this and set the flag?

Comment 20

[[:fabrice] Fabrice Desré]

Paul, I'm a bit confused by your comments. Do you see issues only when sideloading apps, or also when doing a normal installation?

Comment 21

[[:fabrice] Fabrice Desré]

Attachment: sideload-csp.patch

Adding csp support for sideloaded apps. Also updated tests for both packaged and hosted apps.

Comment 22

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Fabrice Desré [:fabrice] from comment #20)
> Paul, I'm a bit confused by your comments. Do you see issues only when
> sideloading apps, or also when doing a normal installation?

Sorry its been a while and my comments are confusing but I think it was for both. I installed the arrow app, which I expected to have a more strict CSP than the default one, but it didn't seem too. In comment 17, i meant that the manifest contains a CSP, but when looking in webapps.json, the csp attribute was blank. If this test is accurate, but if it is the case I suppose its separate issue to the issue of the sideloading not applying a CSP.

But I will retest tomorrow to make sure (leaving need-info to remind me).

Comment 23

[Alexandre Poirot [:ochameau]]

Comment on attachment 8486876 [details] [diff] [review]
sideload-csp.patch

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

Thanks!

Comment 24

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/b2g-inbound/rev/faa9786636cd

Comment 25

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Ok so this seems to work, but I think I found a bug with CSP. But just to record my test result, I installed an app using web-ide, which had the following in the manifest:

...
"type": "certified",
  "csp":"img-src 'self'; script-src 'self'; style-src 'self';"
...

Looking at webapps.json, I can see that this custom CSP has been recorded correctly:

"3e56c15c-e7cc-9c43-84ab-ac40f8fe9593": {
    "origin": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
    "installOrigin": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
    "manifestURL": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593/manifest.webapp",
    "appStatus": 3,
    "receipts": [],
    "kind": "packaged",
    "installTime": 1410501162287,
    "installState": "installed",
    "removable": true,
    "id": "3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
    "basePath": "/data/local/webapps",
    "localId": 1034,
    "name": "CSP Test",
    "csp": "img-src 'self'; script-src 'self'; style-src 'self';",
    "role": "",
    "redirects": null
  }


The issue is that even with this CSP applied, this page still loads an external image:
https://github.com/pauljt/csptest/blob/master/image.html

Comment 26

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I raised 1066462 for this issue.

Comment 27

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Note that I also tested this directly by loading these pages on a webserver the image was in fact blocked in a web page loaded in the browser on the same phone.

Comment 28

[[:fabrice] Fabrice Desré]

That landed on m-c: https://hg.mozilla.org/mozilla-central/rev/faa9786636cd

Comment 29

[Ryan VanderMeulen [:RyanVM]]

I guess you can self-approve this for Aurora :)

Comment 30

[[:fabrice] Fabrice Desré]

Comment on attachment 8486876 [details] [diff] [review]
sideload-csp.patch

Approval Request Comment
[Feature/regressing bug #]: Missing feature
[User impact if declined]: Users could sideload apps that access resources they should not get.
[Describe test coverage new/current, TBPL]: xpchsell tests.
[Risks and why]: No special risk, we're hardening app sideloading support.
[String/UUID change made/needed]: n/a

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c7477d4b33e7