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
setPointerCapture should be disabled in sandboxed iframes by default #16
Comments
This is related to this discussion about the correct default behavior for Mouse Events |
I suppose the reason the spec doesn't say this is because there isn't any specialized limitations that I'm aware of. This happens to work because:
So:
I think, perhaps, we could clarify that when capture is set then document-relative properties (e.g. pageX, pageY) continue to refer to the owner document of the capture target node. Technically, these properties already say they refer to event.target's doc, but it could be made explicit for clarity. Do you agree with this behavior? Is there more clarification (normative or non-normative) you think we should add? |
Thanks, this mostly sounds good to me (I love the lack of special cases here). But I'm worried it's not quite enough to maintain security properties I thought were important (so figured there was probably some special case in implementations here). In particular, if the assignment of pointer IDs is predictable, then this allows an iframe to steal input events for touches that are nowhere near it. So, for example, an ad sandboxed in an iframe could cause any tap/click on the page to open a pop-up of it's choosing. In some cases, the pattern of mouse/touch events themselves may be sensitive - eg. for a site that has a pin-pad for entering your password - an iframe shouldn't have access to those input events, right? |
The IDs for touch are often small and monotonically increasing in Edge (and it's of course a constant for mouse), so I think there is a legitimate attack possible here (eg. could probably even try calling Here's a couple possible mitigations:
|
In the Edge implementation (I tested with IE11) another frame can indeed steal the input from the outer frame. Particularly in this sample you can see that another iframe is stealing the mouse input (knowing the id is always 1 for mouse or I guess it could have randomly tests thousands ids as you suggested) from the outer frame in IE 11. http://output.jsbin.com/nadoxi All you need to do is clicking outside of the iframe. Beside the stealing there are other issues as well like although the click is released the pointer is captured but they are not related to the issue of stealing the input. |
We discussed this today on the call. It sounds like there's agreement that at a minimum sandboxed iframes shouldn't permit capture from outside their frame in order to prevent user annoyance, in the same way that they don't permit pointer lock by default. However it's not clear whether there is real privacy/security risk here in practice. There are a number of reasons this is hard to usefully exploit. Discussion of this is moving to this private issue to permit concrete discussion of potential attack vectors. |
Summary: it sounds like everyone agrees that the security risk is pretty low but that sandboxed iframes should have some mitigation to prevent user annoyance due to malicious (eg. attention grabbing) iframes. So the outstanding question is, is there a minimal mitigation we can apply that doesn't break any reasonable use case that we should just apply universally rather than need to special case sandboxed iframes. @teddink says he'll talk to more MS folks and follow up with details on the security bug. |
I'm now convinced that, although we should probably do something here, it's probably not urgent. Likely worst case is user annoyance, and so this only really becomes urgent if frames start abusing this. Removing the v2-blocking label. |
I think the main thing we need to do here is have setPointerCapture fail by default inside a sandboxed iframe, and have a new |
I would suggest that element.setPointerCapture(id) should only be possible in response to an event dispatched to that element from pointer#id. This solves the iframe and even same document issues ensuring that capture is only occurring for elements that are being interacted with. In the rare cases where a developer may need to capture events from an area distant from the control they are conceptually related to then the event handler must be placed at some parent node, e.g. the document.body. If that suggestion is followed, then I do not think a sandbox attribute for allow-pointer-capture is necessary because the iframe contents will only be able to enter capture when they are interacted with and capture will be released when user interaction ceases (pointer exits the active buttons state). |
@schieb: thanks for the suggestion. We discussed a much more permissive (but more hacky) form of that restriction in the chromium bug and Microsoft folks were concerned that it would break some legitimate use cases (eg. where one frame One idea is to add a metric to Chrome to track what fraction of Thoughts? |
I do think the cross frame capturing will be a concern (certainly spammy advertisements would love to be obnoxious to containing frames, there's ample evidence of them doing anything possible). So I support closing that. Using the sandbox tag is a solution, but seems heavier weight than necessary, and won't solve non-sandboxed iframes. It passes complexity to the web app developers and in some cases would be easily overlooked (it's opt in). I continue to think the "only setPointerCapture from an event to that element" is much cleaner, deals with the issues without needing web developer involvement. I think the rare cases capture is needed to work around that are still possible, e.g. by the methods I and you just cited. A search for "resize iframe interactively" found a few hits and demos (both simple html5 and jquery), so I don't think your last example is a reason to complicate capture. However, if folks did want to pursue the sandbox with allow-pointer-capture, I don't see other reasons not to. |
@RByers (mainly because you were so active on this, I know it's not directly your area anymore), @smaug---- @NavidZ any thoughts on this? any change required at the PE spec end? |
I think others convinced me it wasn't as serious of an issue as I feared. I'd be OK calling this WontFix unless/until we see someone actually exploiting it... |
I agree. |
We could also really easily define a feature policy for this now - to at least allow embedders to turn this off. /cc @clelland |
we'd need to decide if anything needs adding/mentioning (even in a non-normative note) in the spec, or if we want to defer this to future-v3 |
I talked again with the folks here and as I said in the call they don't see a security risk here (also mentioned in that private chromium bug with MS people and others). There might be an abuse of the API that may cause some user annoyance but we suggest to keep things as is and if that becomes a thing then we add more restrictions to the API as this is the pattern in the web platform for potential user annoyance from an API. I added future-v3 for now to keep and eye on this issue. I'm happy to discuss this further with other security folks @plehegar. |
Discussed in PEWG call today. https://lists.w3.org/Archives/Public/public-pointer-events/2020JulSep/0062.html Closing this, with understanding that as it's in the security tracker, we'll get this flagged again if it's a concern. |
The algorithm for setPointerCapture says nothing about different iframes.
If, for example, code running in some iframe happens to call setPointerCapture for a pointer ID that would otherwise have its events sent to another frame, then setPointerCapture should presumably fail somehow (eg. to prevent input stealing). How exactly should this fail?
What about more subtle scenarios? Eg. if I mouse down over one frame, can it capture events so that when I drag out of that frame events consider to go to it?
Also, Is there any reason to prevent a child frame from explicitly directing capture to ancestor frame somehow? I've seen sites do this today by delegating all events inside a child into their parent via postMessage (eg. to allow dragging on a window resize handle within the frame to resize the frame).
The text was updated successfully, but these errors were encountered: