whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.09k stars 2.66k forks source link

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

Open dvoytenko opened 7 years ago

dvoytenko commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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

clelland commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@bzbarsky you might find this interesting.

bzbarsky commented 6 years ago

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 commented 6 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 7 months ago

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 https://github.com/w3c/webappsec-permissions-policy/issues/349#issuecomment-1969345987 for details.

WebReflection commented 1 week ago

same (or very similar) issue flagged as duplicated but the word security is not present in here so I'd like to bring that concern up too, as described in here: https://github.com/whatwg/dom/issues/1315#issue-2572495204

juj commented 1 week ago

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 https://github.com/whatwg/html/issues/10687.