search

WICG / scroll-to-text-fragment

Proposal to allow specifying a text snippet in a URL fragment
Other
589 stars 42 forks source link

Prevent invocation from popup #64

Closed bokand closed 4 years ago

bokand commented 4 years ago

For security, we want to avoid invoking the text directive on windows that can be scripted by another origin. It was pointed out in https://github.com/w3ctag/design-reviews/issues/392#issuecomment-510855073 that our current set of restrictions doesn't work in all cases, an attacker could:

Since V is top-level and has no opener, origin A is now able to cause text directive invocations in V which is bad because it persists past the navigation.

bokand commented 4 years ago

In the tag review thread, @annevk says:

Did you consider only allowing this cross-origin if the link/popup has noopener semantics?

Do you mean only allow the text directive to invoke if the source link/script-context is in a popup window that was opened with rel=noopener? If so, I think might work. I wonder if we could do even simpler and better: in Navigating Across Documents we always have a source browsing context. We could only allow invoking the feature if source browsing context is browsing context, I think that (combined with the top-level browsing context restriction) mean we wouldn't invoke in these popup <--> top window cases.

That said, I think you're still vulnerable to something like this:

I think it's still worth doing since this adds another hurdle for a successful exploit but I'd also point out this alone isn't a vulnerability. An attacker still needs a user gesture and is still blocked by SOP from learning anything interesting about the target origin and a popup is more likely to alert a user to suspicious activity or be blocked.

@annevk WDYT?

annevk commented 4 years ago

A variant I had in mind is that you can only use this syntax with <a>/<area> and window.open(), to prevent that issue. It does seem useful to be able to open a new window and use this feature so forbidding that completely I'm less sure about. And it would be quite easy to circumvent anyway by opening a new window with noopener to something you control and then navigating that.

bokand commented 4 years ago

A variant I had in mind is that you can only use this syntax with / and window.open(), to prevent that issue

Do you mean that in all cases it should only invoke the text directive if the navigation came from an <a>/<area> or a window.open with noopener set? Is this what's meant by "noopener semantics"?

I think there's probably some useful cases for being able to navigate from script (e.g. building custom buttons/controls) but that's probably an ok tradeoff? I was worried an attacker could still use target= but trying it out I learned that noopener restricts targets to blank windows.

annevk commented 4 years ago

They could use target if there's an existing window with that name, but in that case this ought to fail, I think. Basically, only if you end up with the noopener effect you get to navigate something cross-origin in this way. For Google Search that might require having some kind of noopener for _self links however.

bokand commented 4 years ago

Ok, that sounds like a reasonable restriction to me. Is there any existing terminology relating to this in the HTML spec I could leverage or should I spell it out explicitly?

annevk commented 4 years ago

HTML has some noopener infrastructure already, see "The rules for choosing a browsing context" in particular. As I mentioned, we might have to add noopener support for _self links to make it truly useful for this.

bokand commented 4 years ago

Quick clarification:

Basically, only if you end up with the noopener effect you get to navigate something cross-origin in this way.

By "the noopener effect" do you mean that a new top-level browsing context is created? I.e. that we end up in a new browsing context group?

As I mentioned, we might have to add noopener support for _self links to make it truly useful for this.

Based on the above, does this mean that a _self link would create a new top level browsing context? Doesn't that mean we'd lose history navigation?

And a somewhat tangential question based on the target comments above:

They could use target if there's an existing window with that name, but in that case this ought to fail, I think.

By "this ought to fail" you mean activating the text directive, right?

In the MDN page for window.open it says:

Note that when noopener is used, nonempty target names other than _top, _self, and _parent are all treated like _blank in terms of deciding whether to open a new window/tab.

I see this behavior in Firefox; however, in Chrome, if you open a window with name and without noopener, you can in fact now window.open into name even if you specify noopener. The result does have a window.opener which seems...wrong?

I don't see where in the spec the MDN text comes from but it seems wrong that opening a link with 'noopener' can result in the target having window.opener. Is this expected?

annevk commented 4 years ago

Yeah, a new browsing context group. I don't think this should mean that history is lost (if so COOP could destroy history), but it likely requires changes to that code, yes.

By "this ought to fail" you mean activating the text directive, right?

Yes.

Is this expected?

Unfortunately, yes. Firefox has an open bug. We want the same behavior for noopener and noreferrer and couldn't change the behavior for noreferrer anymore. So if you want the noopener effect you should use _blank (which also happens to default to noopener, though not universally so yet).