444728 - autocomplete disregards maxlength for input fields

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Vassil Hristov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9) Gecko/2008052912 Firefox/3.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9) Gecko/2008052912 Firefox/3.0

It is possible to bypass the maximum length limitation for text input fields with autocomplete. Autocomplete offers any text, even if it's longer than what is allowed and the field is consequently populated after selecting the too long text.

Reproducible: Always

Steps to Reproduce:
1. Create a new html file with following content in the body:
<form id="testForm" method="GET">
	<input id="testInput" type="text" />
	<input type="submit" />
</form>
2. Open the file in the browser and type "0123456789" in the input field and hit submit.
3. Change the file and add 'maxlength="5"' to the input field.
4. Go back to the browser and refresh.
Actual Results:  
Now it is possible to select "0123456789" as value for the input field.

Expected Results:  
Autocomplete should not render suggestions that are longer than _maxlength_ or when such a value is selected, it should be trimmed to a total length of _maxlength_.

I believe this is quite a critical issue, as most developers rely on the size of the strings that are provided by the limited input fields. That is - many applications probably would behave in an unexpected manner, when provided with longer texts.

Comment 1

[Johnathan Nightingale [:johnath]]

Hunh - no doubt about it, confirmed via data url.

I don't think this needs to be hidden, there are plenty of ways to get around maxlength parameters and they aren't something web developers should ever rely on as a safety mechanism; they only keep honest people honest, basically.  The exploit possibilities seem somewhat remote, and only make slightly more visible an existing vulnerability in the target website (i.e. relying on maxlength).

Nevertheless, we should fix it, and I'm surprised it hasn't come up earlier, but I'm not having any luck finding an existing bug.  Bug 204506 is similar, but clearly didn't fix this problem.  Bug 443363 is a dup of this bug.

Comment 3

[Daniel Veditz [:dveditz]]

This is a regression from Firefox 2.0.0.x which correctly truncates the autocomplete data at the maxlength.

> I believe this is quite a critical issue, as most developers rely on
> the size of the strings that are provided by the limited input fields.

That would be unwise. Hackers are not constrained by the maxlength limit and would love to find that exceeding it throws your server for a loop.

Mike: did anyone re-work autocomplete for FF3? I assume the awesomebar is its own thing and not built on autocomplete but that could be wrong.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

The autocomplete code doesn't do anything special related to maxlength, so this was probably caused by bug 345267. That change made it possible to enter more than maxlength characters into a text field programmatically, to match other browsers. The autocomplete code sets the value via the same code path as page scripts, so it was affected too.

Comment 5

[Samuel Sidler (old account; do not CC)]

Boris, can you look at this?

Comment 6

[Boris Zbarsky [:bzbarsky]]

This needs a fix on the autocomplete side: it needs to be checking the maxlength.  It didn't need to before, as Gavin said, because it was relying on a core bug.  Then we fixed the core bug.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: WIP

I put this together a while ago but couldn't get the test to work. The entries added by the test are apparently not added correctly because they don't appear as options, and even if they did I'm not sure that the code I use to select the autocomplete entry will work.

Comment 12

[Matthew N. [:MattN]]

Attachment: v.1 only show entries that fit (applies to patch v.3 on bug 446247)

Instead of truncating, only show form history entries that will fit in the field.

Comment 13

[Justin Dolske [:Dolske]]

Comment on attachment 389264 [details] [diff] [review]
v.1 only show entries that fit (applies to patch v.3 on bug 446247)

>+            if (aField && aField.maxLength > -1)
>+                result.wrappedJSObject.entries = result.wrappedJSObject.entries.filter(this._filterEntryLength, aField);

Use an inline anonymous function here...

foo = foo.filter(function (e) { return (element.text.length <= this.maxLength) });

(A refinement of having a local function in the if-block to do this, which would be my choice instead of having a tiny utility function stuck, at  distance, onto the component's object).

Comment 14

[Matthew N. [:MattN]]

Attachment: v.2 inline function & update unit tests

Comment 15

[Justin Dolske [:Dolske]]

Comment on attachment 390343 [details] [diff] [review]
v.2 inline function & update unit tests

sr? for API change, this was added in 3.6 so there's no compat issues.

Comment 16

[Matthew N. [:MattN]]

Attachment: v.3 fix bitrot

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 390676 [details] [diff] [review]
v.3 fix bitrot

sr=mconnor

Comment 18

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/0c9d24a0f828

Comment 19

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090925 Namoroka/3.6b1pre