Open Bug 2006870 Opened 9 months ago Updated 5 months ago

Don't execute onbeforematch in meta tag

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

People

(Reporter: tschuster, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: spec-needed)

Attachments

(1 file)

testcase.html
120 bytes, text/html
Details

Unlike Chrome, Firefox will automatically execute onbeforematch with the following test case:

<meta http-equiv=refresh content=0,#x id=x onbeforematch=alert() hidden=until-found>

Nowadays, working #xss vectors that rely only on a meta tag and don't require user interaction are rare. Here's one that needs no user interaction and no user-supplied URL fragment: the meta tag navigates to #x, firing its own onbeforematch handler.

https://x.com/nowaskyjr/status/2000638463469875680

We should figure out why exactly this alert() doesn't trigger in Chrome and look at the right result according to the specification.

See Also: → 1761043
Attached file testcase.html —
Attachment #9533904 - Attachment mime type: text/plain → text/html

As per the HTML spec, all HTML elements have the hidden attribute, and therefore find-in-page / fragment scrolling will run the ancestor revealing algorithm, which fires beforematch.

I tried to trace through their code if they have implemented any exception list for elements like meta or link. It seems that the onbeforematch attribute is parsed as expected (so is the hidden attribute). Also, beforematch is fired when scrolling to a fragment.

OP of the X repro mentioned in the report here.

Just to add some context: onbeforematch="alert()" not firing on Chrome isn't just in <head><meta>. The same happens on any other element too. You can see the attribute is there (getAttribute("onbeforematch") returns "alert()"), but el.onbeforematch stays null, so there's nothing hooked up to execute when the event fires.

At the same time, both addEventListener("beforematch", …) and el.onbeforematch = fn work fine. Looks like Chrome is just missing the inline handler attribute wiring here. (https://issues.chromium.org/issues/40119768)

Firefox firing it on a <head><meta> still seems like its own bug.

As Jan said, Firefox firing the event follows HTML spec. If some change is wanted, one should file a spec issue.

And onbeforematch attribute should be supported by all the HTML elements
https://html.spec.whatwg.org/#ancestor-revealing-algorithm:~:text=onbeforematch,-HTML

Severity: -- → S4
Priority: -- → P3
Flags: needinfo?(tschuster)

Is there some precedence to limit events like this in the HTML spec or should we just report a Chromium issue?

Flags: needinfo?(tschuster) → needinfo?(jjaschke)

I would assume we could change the spec to stop the ancestor revealing algorithm if ancestors are not being rendered, which AIUI would match HTML elements like <head>, <meta> etc:

(..)
4. For each (ancestorToReveal, revealType) of ancestorsToReveal:

4.1 If ancestorToReveal is not connected, then return.
4.2 If ancestorToReveal is not being rendered, then return.
4.3 If revealType is "until-found":

(..)
4.3.2 Fire an event named beforematch at ancestorToReveal (..)

I haven't thought this through though, especially if we should stop or skip, and I'd like Simon's opinion on this. :)

Flags: needinfo?(jjaschke) → needinfo?(zcorpan)

"being rendered" is easy to work around with the style attribute so doesn't solve the attack. Also the page might want to navigate to a hidden element and make it visible in the beforematch handler.

I guess we could check for meta elements in the ancestor revealing algorithm.

Flags: needinfo?(zcorpan)

Actually checking for meta elements doesn't help much because the event handler could be on any other element. More generally, if refresh navigation is allowed to fragment-navigate, then a lot of events can fire because of that, e.g. load or error on lazy-loading images.

In Chromium it seems the onbeforematch event handler content attribute is not supported (but the event handler IDL attribute is supported).

In WebKit it seems refresh navigation to a fragment doesn't do a fragment navigation but a "regular" navigation.

Chromium's approach is not great for DX nor is it a general defense against causing scripts to run in response to a fragment navigation.

We can maybe standardize WebKit's approach? Filed https://github.com/whatwg/html/issues/12325

WebKit's approach doesn't completely prevent XSS, but

  1. you need a stored injection for the injected event handler to appear also after navigation
  2. it's equivalent to loading the page with a fragment

Text fragments should have some check for user activation iirc, which maybe should prevent them from working from <meta http-equiv=refresh>?

Flags: needinfo?(jjaschke)

Reported a Chromium issue for the non-working onbeforematch attribute: https://issues.chromium.org/issues/500653046

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: