Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent invocation from popup #64

Closed
bokand opened this issue Oct 25, 2019 · 8 comments · Fixed by #70
Closed

Prevent invocation from popup #64

bokand opened this issue Oct 25, 2019 · 8 comments · Fixed by #70

Comments

@bokand
Copy link
Collaborator

bokand commented Oct 25, 2019

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

  • Top-Level window A1 Opens a popup A2
  • popup A2 navigates A1 to V with :~:text

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
Copy link
Collaborator Author

bokand commented Oct 25, 2019

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:

  • A1 opens A2
  • A1 navigates itself to V
  • A2 still has a window.opener ref to the top-browsing context so it can window.opener.location = A1 to navigate back and can repeat the process.

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
Copy link
Collaborator

annevk commented Oct 25, 2019

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
Copy link
Collaborator Author

bokand commented Oct 25, 2019

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
Copy link
Collaborator

annevk commented Oct 29, 2019

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
Copy link
Collaborator Author

bokand commented Oct 31, 2019

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
Copy link
Collaborator

annevk commented Nov 4, 2019

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
Copy link
Collaborator Author

bokand commented Nov 7, 2019

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
Copy link
Collaborator

annevk commented Nov 8, 2019

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants