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

Provide an opt-out for inputs overriding form DOM API #2212

Open
dvoytenko opened this issue Dec 22, 2016 · 12 comments
Open

Provide an opt-out for inputs overriding form DOM API #2212

dvoytenko opened this issue Dec 22, 2016 · 12 comments
Labels
integration Better coordination across standards needed

Comments

@dvoytenko
Copy link

The problem involves how HTML FORM inputs are exposed on the parent <form> element. It was originally introduced in the MS Internet Explorer as far back as JavaScript 1.0 and eventually copied by most of browsers. W3C DOM Level 2 spec has addressed this by introducing HTMLFormElement.elements collection. Unfortunately, the old behavior was left intact for backward compatibility.

Consider the following markup:

<form action="...">
  <input name="submit">
</form>

Normally, you can submit the form via form.submit() API. However, b/c input with name "submit" is present on the form, it overrides form.submit to be a reference to this input. Thus, form.submit() will throw an error "form.submit is not a function".

The same as true for any of the form's properties or methods. As the bottom-line, any normal DOM API can be overridden this way on the form. This, at best, is a big source of inconvenience, and, at worst, an XSS vector.

This old feature is neither needed nor expected. The spec already has form.elements collection to provide this functionality w/o messing up the form's APIs. It'd be nice to opt-out a form or a whole page out of this behavior. For instance, <form do-not-expose-inputs-on-form> style, or any other in markup or JS API.

You can read more about this problem in this post: https://medium.com/@dvoytenko/solving-conflicts-between-form-inputs-and-dom-api-535c45333ae4

@zcorpan
Copy link
Member

zcorpan commented Dec 23, 2016

This is the behavior of https://heycam.github.io/webidl/#OverrideBuiltins which, in HTML, is set on:

https://html.spec.whatwg.org/#document
https://html.spec.whatwg.org/#domstringmap
https://html.spec.whatwg.org/#htmlformelement

DOMStringMap is only used by element.dataset, and is probably not a problem.

Other interfaces that don't have OverrideBuiltins but still supports named properties:

https://html.spec.whatwg.org/#named-access-on-the-window-object
https://html.spec.whatwg.org/#plugins-2
https://html.spec.whatwg.org/#the-storage-interface

Plugins and Storage are probably not a problem, but certainly polluting the global is a problem...

@annevk
Copy link
Member

annevk commented Jan 22, 2018

Maybe this can be controlled via a feature policy? Would end up being a little tricky to define, but it would give some rather nice benefits.

cc @clelland @domenic

@annevk annevk added the integration Better coordination across standards needed label Jan 22, 2018
@clelland
Copy link
Contributor

This would be a good wart to remove -- does the frame-scope of feature policy make sense? @dvoytenko 's original proposal was per-form; FP would ensure that all forms on a page (and any nested content) behave the same way.

I expect that for compatibility with the existing web, the feature would have to be defined as "expose inputs on form", and enabled by default for all content. But once turned off at any level, feature policy would ensure that it remained off for that frame and any nested frames.

@annevk
Copy link
Member

annevk commented Jan 23, 2018

It definitely makes sense for the Document and Window objects, which I think we should include in this kind of switch (not sure about DOMStringMap). And OP seems fine with it covering all form elements.

@annevk
Copy link
Member

annevk commented Jan 23, 2018

@bzbarsky you might find this interesting.

@bzbarsky
Copy link
Contributor

Hmm. This adds some implementation complexity, but I agree that it makes the DOM APIs saner to use...

For Window this is less of an issue, because Window is not [OverrideBuiltins]. For Document I wonder whether we could stop doing [OverrideBuiltins]. Last I checked IE wasn't doing it there, not sure about Edge. But Chrome at the time had an architecture that didn't really allow the non-overridebuiltins behavior for Document; I'm not sure what their situation is now...

(Of course changes to existing behaviors would have more compat risk than adding a toggle, but end up with a simpler implementation and platform, and no confusing "yeah, actually, behave sanely" toggle.)

@cdumez
Copy link

cdumez commented Feb 7, 2018

I am curious how much breakage there would be from simply dropping this behavior. It might be worth a try.

At the very least, I think it would be great to have a way to disable it.

@mikewest
Copy link
Member

@cdumez: Chrome's dataset shows usage on a bit more than 8% of page views. It's difficult to tell how much user-visible breakage would result from changing the behavior, but 8 is a lot of percent, well above the bar Blink has successfully deprecated in the past. Perhaps there are smaller pieces we could turn off by default, or heuristics we could divine by digging into HTTP Archive data, but I think that's going to be a lot of work. Something in FP seems like a more certain approach.

@bzbarsky
Copy link
Contributor

@mikewest Is that the right metric? I only see two places incrementing that counter in https://cs.chromium.org/search/?q=DOMClobberedVariableAccessed+file:%5Esrc/+package:%5Echromium$&type=cs and both are in the bits that implement the Window's named properties object stuff. Those aren't measuring either named access on the document or named access on forms, and they are definitely not measuring whether the access shadows anything or not, right?

@securityMB
Copy link

securityMB commented Feb 28, 2024

Those aren't measuring either named access on the document or named access on forms, and they are definitely not measuring whether the access shadows anything or not, right?

@bzbarsky A new CL just landed in Chrome which will measure named access on both document and forms. See w3c/webappsec-permissions-policy#349 (comment) for details.

@WebReflection

This comment was marked as outdated.

@juj
Copy link

juj commented Oct 8, 2024

Given that this ticket #2212 discusses an opt-out, a general solution and is in the process of gathering user statistic, I opened a separate ticket to specifically discuss the narrower security-relating part as a new ticket at #10687.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Better coordination across standards needed
Development

No branches or pull requests

10 participants