support javascript: protocol for longdesc attributes

UNCONFIRMED
Unassigned

Status

()

Firefox
Menus
--
major
UNCONFIRMED
2 years ago
11 months ago

People

(Reporter: ytrezq, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Created attachment 8658895 [details]
Test file for reproducing this bug

Currently, a javascript uri used in a longdesc attribute does nothing the image description is triggered from the context menu :
No error is shown in developers tools, noting appends. It is as if the longdesc attribute were empty.
Other uri shemes are shown.

Some web browsers support it. Presto Opera opened javascript: uri in a separate tab.

At least if the community refuse to support it, the best would be to throw an error in the console for developer tools : I spend a lot of time checking my syntax ꜱᴏᴘ and many other things like that (because I thought it did came from an error of mine).

Updated

2 years ago
Component: JavaScript Engine → DOM
(Reporter)

Updated

2 years ago
Depends on: 877453
This is handled completely in the UI, not in the rendering engine.

For the record, I don't think we should support javascript: here; that would lead to script injection attacks.  Reporting an error to the error console is probably OK.
Component: DOM → Menus
Product: Core → Firefox
(Reporter)

Comment 2

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #1)
> For the record, I don't think we should support javascript: here; that would
> lead to script injection attacks.

I can’t see how this is related to script injection. We are already supporting the JavaScript protocol in other elements.
A consistent sanitizer should already restrict protocols for other attributes (like href with <a> or link).

I can’t imagine adding an attribute to a Sanitizer whitelist without looking on how it does and how it works.
A blacklist based Sanitizer would definitely leave other open vector for that kind of attack.

Just the matter of staying consistent : either allow the JavaScript protocol in attributes or don’t do it at all !
Component: Menus → DOM
Product: Firefox → Core
(Reporter)

Updated

2 years ago
Depends on: 656433
> We are already supporting the JavaScript protocol in other elements.

The only elements that support it in the rendering engine at this point are <frame>, <iframe>, <object>, and <a>.

I'm not sure why you decided to move this back to the wrong product....
Component: DOM → Menus
Product: Core → Firefox
(Reporter)

Updated

2 years ago
Depends on: 1203457
(Reporter)

Comment 4

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #3)
> I'm not sure why you decided to move this back to the wrong product....
Because I guess that part is for implementing an error message

It is possible to allow it like presto based opera did (so it couldn’t allow XSS). Or just do like the <a> element : display the target address in the browser bottom when the mouse is over the button for clicking to lead to the destination address.

I now realize this touch a broader issue so I opened a separate bug.
> Because I guess that part is for implementing an error message

The error message would need to be shown by the code that blocks the load, which is the front-end code.

> It is possible to allow it like presto based opera did

It's possible, but not very useful, since most javascript: URIs would operate on an existing page.  What, exactly, are the use cases for javascript: longdesc where the script runs in a completely new blank page?
(Reporter)

Comment 6

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #5)
> > Because I guess that part is for implementing an error message
> 
> The error message would need to be shown by the code that blocks the load,
> which is the front-end code.
And I am saying the security reason for implementing an error over the JavaScript protocol is completely nonsense here.
— javascript: ᴜʀɪ are allowed in several other tags. Some of them will execute scripts without requiring user interaction. The user interaction required for triggering the script involve using a context menu, not very ideal vs the <a> element.
— data: ᴜʀɪ with the longdesc attribute is allowed. So an attacker can still perform ᴄʀꜱꜰ/session hijacking with that attribute without requiring to bypass SOP (remember #1016491). This ᴜʀɪ scheme is supported by all implementations which render the longdesc attribute.
— Internet explorer and Google chrome allow the javascript: protocol on longdesc.
— Ok, an attacker that still want using the longdesc attribute even if other methods are available for XSS would require several images in order to get cross‑browsers support. but I recognize a bit of social engineering might solve this.

In order to require an attacker using the longdesc attribute with the JavaScript protocol, such hypothetical comment sanitizer should definitely be poorly written. It would need to :
— blacklist protocols correctly on <frame> ; <iframe> ; <object> ; <a>. Or either don’t allow them (seems problematic for <a>)
— Whitelist the longdesc attribute without really investigating what are the risks.
— Don’t reuse the code necessary to strip protocols (re-using existing code is far easier than writing new ones like sanitizing ꜱᴠɢ) so that data: ᴜʀɪ and all scripting protocols are stripped except javascript: .

If you find such a widely used sanitizer, then please send a link. Currently I think only someone with no experience in coding can write something like this for his very low traffic blog (and thus is susceptible to open other attack vectors in the future).
Either it is possible to exploit it and other other more accessible attack vectors are open (say no striping). Or other XSS attack vectors aren’t available and the javascript: protocol is filtered. So the current behaviour only prevent legitimate use.
I thought without gecko’s community consensus ᴡ3ᴄ recommendations (whose redaction are also partially community base) should apply (there are no specific behaviour for ᴜʀɪ type restriction).
I recognize everything is not perfect in ʜᴛᴍʟ4 or ʜᴛᴍʟ5 and the current javascript: behaviour can lead to XSS.

But if objecting ᴡ3c recommendation then let’s do it right ! Need to get a clear gecko’s community consensus for disabling the javascript: protocol completely inside ʜᴛᴍʟ documents (it would remains available in the navbar).
This is the matter to be discussed elsewhere than on this page like on irc://irc.mozilla.org#developers or in an other issue like this one : https://bugzilla.mozilla.org/show_bug.cgi?id=1203457

> What, exactly, are the use cases for
> javascript: longdesc where the script runs in a completely new blank page?
You can try the attachment in Presto based opera or in Internet explorer >8.  It still offers a very good reason to allow JavaScript with the longdesc attribute outside of the current document.

If javascript: on the current document would be allowed, An useful use would be to replace dynamically the image with description text when viewing is triggered. This is one of the reason I choose the title of this bug over implementing an error message for longdesc scripting.
Component: Menus → DOM: Core & HTML
Product: Firefox → Core
Please stop moving this into DOM.  The code you want changed isn't in the DOM.  If you move it here again, I will resolve the bug as invalid, because as a bug against the DOM implementation it _is_ invalid.  There is no problem with the DOM implementation here.

> It would need to :
> — blacklist protocols correctly on <frame> ; <iframe> ; <object> ; <a>. Or either don’t
> allow them (seems problematic for <a>)

Any sane sanitizer uses a whitelist and this whitelist does not include <frame>, <iframe>, or <object> at all.

<a> will be on the whitelist, but will have its href checked against a scheme whitelist.

And yes, the worry is whitelisting the longdesc attribute without considering the risks.
Component: DOM: Core & HTML → Menus
Product: Core → Firefox
(Reporter)

Comment 8

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #7)
> Please stop moving this into DOM.  The code you want changed isn't in the
> DOM.  If you move it here again, I will resolve the bug as invalid, because
> as a bug against the DOM implementation it _is_ invalid.  There is no
> problem with the DOM implementation here.
Ok it was because :jandem marked it as ᴅᴏᴍ, so I thought it was the relevant part for implementing JavaScript (not the error message).
> Any sane sanitizer uses a whitelist and this whitelist does not include
> <frame>, <iframe>, or <object> at all.
Correct
> <a> will be on the whitelist, but will have its href checked against a
> scheme whitelist.
Like src on images and some kind of other attributes (gecko isn’t the most used rendering engine). Create a single function or a class for whitelisting protocols in order to strip data: ᴜʀɪ and all scripting protocols.
> And yes, the worry is whitelisting the longdesc attribute without
> considering the risks.
Ok, but by simply whitelisting without considering the risk, you will probably don’t strip protocols at all for that attribute and let the data: ᴜʀɪ be used (gecko allow to load data: ᴜʀɪ on longdesc attributes). Currently, an html document loaded from the data: ᴜʀɪ inherit the origin policy of the precedent document like javascript: (remember https://bugzilla.mozilla.org/show_bug.cgi?id=1016491).

So a sanitizer vulnerable to this risk would have to rewrite the protocol sanitization code for each attributes instead of using a single function or class for ruling them all (a bit of https://en.wikipedia.org/wiki/Don't_repeat_yourself).
So that protocols are filtered correctly on all elements except on longdesc where data: ᴜʀɪ would be filtered but not javascript:
Really, seems a far‑fetched scenario for a serious cite with a wide base of users.

Now, you may want to report an issue for disabling the data: ᴜʀɪ on the longdesc attribute, but as I said, this touch a broader issue and should discussed with other community members (https://bugzilla.mozilla.org/show_bug.cgi?id=1203457).
(Reporter)

Comment 9

2 years ago
I also asked whatwg about it and it seems it’s ok with https://html.spec.whatwg.org/multipage/browsers.html#javascript-protocol but :

“ytrezq: longdesc is obsolete, any implementation that supports it is wrong“

“avoid javascript:, the only reason it's in the spec at all is because we cannot break the web”

Comment 10

2 years ago
(In reply to ytrezq from comment #9)
> I also asked whatwg about it and it seems it’s ok with
> https://html.spec.whatwg.org/multipage/browsers.html#javascript-protocol but
> :
> 
> “ytrezq: longdesc is obsolete, any implementation that supports it is wrong“
> 
> “avoid javascript:, the only reason it's in the spec at all is because we
> cannot break the web”

longdesc is not obsolete. It is a W3C Recommendation: 
http://www.w3.org/TR/html-longdesc/
(Reporter)

Comment 11

2 years ago
(In reply to Laura Carlson from comment #10)
> longdesc is not obsolete. It is a W3C Recommendation: 
> http://www.w3.org/TR/html-longdesc/
I just quoted what the guy said

Also note that Google is also opening the javascript: schme in new tabs. (they have official support)
Comment hidden (spam)
You need to log in before you can comment on or make changes to this bug.