Ben Turner (not reading bugmail, use the needinfo flag!) Reporter
	Description • 12 years ago

From discussion with dougt, the "desktop-notification" permission is using nsIContentPermissionPrompt and thereafter able to do whatever it wants. A hacked child could thus send notifications any time and we should add some security checks in the parent process.

We could also sanitize url against sms/tel/etc in case that causes problems.

	Doug Turner (:dougt)
	Comment 1 • 12 years ago

discussed this issue at triage this morning and we agreed that we should block on these sorts of permission bugs for now.

gregor, can you take a look?

	Andrew Overholt [:overholt]
	Comment 2 • 12 years ago

Setting priority based on triage discussions.  Feel free to decrease priority if you disagree.

	Alex Keybl [:akeybl]
	Comment 3 • 12 years ago

Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.

	Dietrich Ayala (:dietrich)
	Comment 4 • 12 years ago

Been nearly 10 days since an update. Gregor, can you update the bug with your status?

	Andrew Overholt [:overholt]
	Comment 5 • 12 years ago

I confirmed with Gregor on IRC that this is next in his list to fix.

	Gregor Wagner [:gwagner] Assignee
	Comment 7 • 12 years ago

Comment on attachment 693184 [details] [diff] [review]
patch

Haven't tested on the phone yet (forgot them at home) but it would be good to know if this is the right direction.

	Gregor Wagner [:gwagner] Assignee
	Comment 8 • 12 years ago

Andrea, would this work with the patch from bug 811026?

	[:fabrice] Fabrice Desré
	Comment 9 • 12 years ago

Comment on attachment 693184 [details] [diff] [review]
patch

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

::: b2g/chrome/content/shell.js
@@ +741,5 @@
>                            uid, name, null);
>    },
>  
> +  receiveMessage: function alert_receiveMessage(aMessage) {
> +    if (!this.assertPermission(aMessage, "desktop-notification")) {

[JavaScript Error: "this.assertPermission is not a function" {file: "chrome://browser/content/shell.js" line: 739}]

If I change it to aMessage.target.assertPermission("desktop-notification") this works. Does it look ok to you?

	Gregor Wagner [:gwagner] Assignee
	Comment 10 • 12 years ago

(In reply to Fabrice Desré [:fabrice] from comment #9)
> Comment on attachment 693184 [details] [diff] [review]
> patch
> 
> Review of attachment 693184 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/chrome/content/shell.js
> @@ +741,5 @@
> >                            uid, name, null);
> >    },
> >  
> > +  receiveMessage: function alert_receiveMessage(aMessage) {
> > +    if (!this.assertPermission(aMessage, "desktop-notification")) {
> 
> [JavaScript Error: "this.assertPermission is not a function" {file:
> "chrome://browser/content/shell.js" line: 739}]
> 
> If I change it to aMessage.target.assertPermission("desktop-notification")
> this works. Does it look ok to you?

Yes that's the right function. I shouldn't post untested code. Sorry for that.

	Andrea Marchesini [:baku]
	Comment 12 • 12 years ago

(In reply to Gregor Wagner [:gwagner] from comment #8)
> Andrea, would this work with the patch from bug 811026?

Yes. It should work.

	Gregor Wagner [:gwagner] Assignee
	Comment 13 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4925a90ee7ff

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 14 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/4925a90ee7ff

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 12 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/38c4ee091bfb
https://hg.mozilla.org/releases/mozilla-b2g18/rev/af4ece87dfb8