Ban inputs with special names

[jridgewell]

As a result of #6540 (comment), here's a list of 287 inherited properties on HTMLFormElements that get overridden by their inputs.

On these elements:

• <button>
• <fieldset>
• <input>
• <keygen>
• <object>
• <output>
• <select>
• <textarea>

We're gonna need to ban these names or ids values...

• ALLOW_KEYBOARD_INPUT
• ATTRIBUTE_NODE
• CDATA_SECTION_NODE
• COMMENT_NODE
• DOCUMENT_FRAGMENT_NODE
• DOCUMENT_NODE
• DOCUMENT_POSITION_CONTAINED_BY
• DOCUMENT_POSITION_CONTAINS
• DOCUMENT_POSITION_DISCONNECTED
• DOCUMENT_POSITION_FOLLOWING
• DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC
• DOCUMENT_POSITION_PRECEDING
• DOCUMENT_TYPE_NODE
• ELEMENT_NODE
• ENTITY_NODE
• ENTITY_REFERENCE_NODE
• NOTATION_NODE
• PROCESSING_INSTRUCTION_NODE
• TEXT_NODE
• __defineGetter__
• __defineSetter__
• __lookupGetter__
• __lookupSetter__
• __proto__
• acceptCharset
• accessKey
• accessKeyLabel
• action
• addEventListener
• after
• animate
• append
• appendChild
• assignedSlot
• attachShadow
• attributes
• autocomplete
• baseURI
• before
• blur
• checkValidity
• childElementCount
• childNodes
• children
• classList
• className
• click
• clientHeight
• clientLeft
• clientTop
• clientWidth
• cloneNode
• closest
• compareDocumentPosition
• constructor
• contains
• contentEditable
• contextMenu
• createShadowRoot
• dataset
• dir
• dispatchEvent
• draggable
• elements
• encoding
• enctype
• firstChild
• firstElementChild
• focus
• getAttribute
• getAttributeNS
• getAttributeNames
• getAttributeNode
• getAttributeNodeNS
• getBoundingClientRect
• getClientRects
• getDestinationInsertionPoints
• getElementsByClassName
• getElementsByTagName
• getElementsByTagNameNS
• getRootNode
• hasAttribute
• hasAttributeNS
• hasAttributes
• hasChildNodes
• hasOwnProperty
• hidden
• id
• innerHTML
• innerText
• insertAdjacentElement
• insertAdjacentHTML
• insertAdjacentText
• insertBefore
• isConnected
• isContentEditable
• isDefaultNamespace
• isEqualNode
• isPrototypeOf
• isSameNode
• lang
• lastChild
• lastElementChild
• length
• localName
• lookupNamespaceURI
• lookupPrefix
• matches
• method
• mozMatchesSelector
• mozRequestFullScreen
• mozRequestPointerLock
• name
• namespaceURI
• nextElementSibling
• nextSibling
• noValidate
• nodeName
• nodeType
• nodeValue
• normalize
• offsetHeight
• offsetLeft
• offsetParent
• offsetTop
• offsetWidth
• onabort
• onanimationend
• onanimationiteration
• onanimationstart
• onbeforecopy
• onbeforecut
• onbeforeload
• onbeforepaste
• onblur
• oncancel
• oncanplay
• oncanplaythrough
• onchange
• onclick
• onclose
• oncontextmenu
• oncopy
• oncuechange
• oncut
• ondblclick
• ondrag
• ondragend
• ondragenter
• ondragleave
• ondragover
• ondragstart
• ondrop
• ondurationchange
• onemptied
• onended
• onerror
• onfocus
• onfocusin
• onfocusout
• oninput
• oninvalid
• onkeydown
• onkeypress
• onkeyup
• onload
• onloadeddata
• onloadedmetadata
• onloadstart
• onmousedown
• onmouseenter
• onmouseleave
• onmousemove
• onmouseout
• onmouseover
• onmouseup
• onmousewheel
• onmozfullscreenchange
• onmozfullscreenerror
• onmozpointerlockchange
• onmozpointerlockerror
• onpaste
• onpause
• onplay
• onplaying
• onprogress
• onratechange
• onreset
• onresize
• onscroll
• onsearch
• onseeked
• onseeking
• onselect
• onselectstart
• onshow
• onstalled
• onsubmit
• onsuspend
• ontimeupdate
• ontoggle
• ontransitionend
• onvolumechange
• onwaiting
• onwebkitanimationend
• onwebkitanimationiteration
• onwebkitanimationstart
• onwebkitcurrentplaybacktargetiswirelesschanged
• onwebkitfullscreenchange
• onwebkitfullscreenerror
• onwebkitkeyadded
• onwebkitkeyerror
• onwebkitkeymessage
• onwebkitmouseforcechanged
• onwebkitmouseforcedown
• onwebkitmouseforceup
• onwebkitmouseforcewillbegin
• onwebkitneedkey
• onwebkitplaybacktargetavailabilitychanged
• onwebkittransitionend
• onwheel
• outerHTML
• outerText
• ownerDocument
• parentElement
• parentNode
• prefix
• prepend
• previousElementSibling
• previousSibling
• propertyIsEnumerable
• querySelector
• querySelectorAll
• releaseCapture
• remove
• removeAttribute
• removeAttributeNS
• removeAttributeNode
• removeChild
• removeEventListener
• replaceChild
• replaceWith
• reportValidity
• requestPointerLock
• reset
• scroll
• scrollBy
• scrollByLines
• scrollByPages
• scrollHeight
• scrollIntoView
• scrollIntoViewIfNeeded
• scrollLeft
• scrollLeftMax
• scrollTo
• scrollTop
• scrollTopMax
• scrollWidth
• setAttribute
• setAttributeNS
• setAttributeNode
• setAttributeNodeNS
• setCapture
• shadowRoot
• slot
• spellcheck
• style
• submit
• tabIndex
• tagName
• target
• textContent
• title
• toLocaleString
• toSource
• toString
• translate
• undefined
• unwatch
• valueOf
• watch
• webkitGetRegionFlowRanges
• webkitMatchesSelector
• webkitRegionOverset
• webkitRequestFullScreen
• webkitRequestFullscreen
• webkitdropzone

Note, the forbidden values list was generated running the following on Firefox, Safari, and Chrome:

const array = [];
let proto = HTMLFormElement.prototype
while (proto) {
  array.push(...Object.getOwnPropertyNames(proto));
  proto = proto.__proto__;
}
const set = new Set(array);

[honeybadgerdontcare]

We currently don't validate for elements <keygen>, <object>, <output>.

[jridgewell]

Yah, I was just giving as exhaustive of a list as I could. It'/ unlikely we'all need to ban every property.

[powdercloud]

In #6540, @mkhatib mentions "...or use getAttribute('id')..." - would this be an alternative to having such a long list, or could it help to trim the list substantially?

[jridgewell]

#getAttribute can be overridden with <input name="getAttribute" />, so it doesn't help us at all. 😢

I think we're going to petition to have this removed from the HTML spec (or at least, the overshadowing of real properties), which is really the only other way to resolve this.

[jridgewell]

Actually, we could ban #getAttribute, then things like class, id, name, etc can be fetched safely through it. But we would still have to ban all of the non-attribute properties (else we'd limit ourselves from future features).

[powdercloud]

Thinking in the long run - maybe it's better to do a whitelist? E.g., if we'd require that values for name and id have to be prefixed with something, they may still be useful enough but avoid these collisions, both now and in the future.

[mkhatib]

I like the idea of prefixed names/ids - @dvoytenko what do you think?

The problem with it though would be if users are using already existing systems and have no control of how they name the inputs this might be painful and very limiting.

[powdercloud]

One more thought is to do some runtime stuff to try to fix up the form. I haven't been successful at this myself thus far, but maybe someone knows better. E.g., I was thinking that if someForm.getAttribute doesn't behave like a function, then I could try to fix up that thing dynamically, by changing someForm.getAttribute.name / someForm.getAttribute.id. Alas, this doesn't seem to work in Chrome thus far - once the name / id has entered the form object it remains stuck in there as a dictionary key. It's possible I tried the wrong thing, just wanted to mention that thought.

[jridgewell]

I was thinking that if someForm.getAttribute doesn't behave like a function, then I could try to fix up that thing dynamically

That's actually in the spec:

Each form element has a mapping of names to elements called the past names map. It is used to persist names of controls even when they change names.

HTML really wanted to screw with devs. We could do something else, though:

• Force all input name/ids to use a prefix "form-".
• When submitting, map them into their non-prefixed values

We'd likely confuse people who are expecting to receive prefixed values, though.

[mkhatib]

I like the runtime prefixing and submitting normally. The problem still exists though when we don't handle submission, I guess we can try to revert the names to be the old prefix before submission just before the submission and hopefully after everything else, this might work.

[powdercloud]

In Chrome something like this appears to work. For a myForm element that has an input child named getAttribute ...
(1) remove the offending input child from the form by using myForm.removeChild; after this myForm.getAttribute is no longer there
(2) assign HTMLForm.prototype.getAttribute to myform.getAttribute; This restores the getAttribute functionality
(3) rename the name on the input node to something else and add it back
I haven't played with step (3) and messing with the dom nodes is heavy handed. Just figure as a proof of concept it's worth posting.
I like the prefix idea better, we'll just have to deal with the forms that are already out there.

[dvoytenko]

I'd like to try the following:

First, we will create proxy object ($proxy) that will proxy selected properties and methods to form instance. It will do so via HTMLFormElement.prototype so that we are not concerned about inputs with the name getAttribute. We will set $proxy object on the form, i.e. form.$proxy. Thus, the proxy will look like:

Object.defineProperty($proxy, 'whitelistedProperty', {
  get: function() {
    // Access the property, e.g. via getAttribute method.
    return Element.prototype.getAttribute.call(this, 'whitelistedProperty');
  }
});

$proxy.whitelistedMethod = function() {
 return HTMLFormElement.prototype.whitelistedMethod.apply(this, attributes);
};

Second, we will ban inputs with the name $proxy.

Third, we will rewrite our binaries on Closure level to do the following replacement via AST:

instanceOfNode.whitelistedProperty ->
   (instanceOfNode.$proxy || instanceOfNode).whitelistedProperty

instanceOfNode.whitelistedMethod() ->
   (instanceOfNode.$proxy || instanceOfNode).whitelistedMethod()

So, this is a complete mess. But some benefits are:

1. It's a mess in the final binary, not in our source code. Our source code continues to nicely call form.id.
2. Access to all properties and methods on most of elements will continue to be fast. Only form access will be slow, but there are fewer forms.
3. We don't need to blacklist anything. Or maybe very few things.
4. If tomorrow there's a way to selectively disable this crazy behavior, e.g. in Chrome, we can do so easily in runtime by simply doing form.$proxy = form.