HTMLSelectElement does not include selected index/indices while saving state

Source/core/html/HTMLSelectElement.cpp

Index: Source/core/html/HTMLSelectElement.cpp
diff --git a/Source/core/html/HTMLSelectElement.cpp b/Source/core/html/HTMLSelectElement.cpp
index 906812120075f2065568cbede9200c54123b4cd9..d0e394a799b910669dc57118b797ae3bc847039d 100644
--- a/Source/core/html/HTMLSelectElement.cpp
+++ b/Source/core/html/HTMLSelectElement.cpp
@@ -1039,6 +1039,7 @@ FormControlState HTMLSelectElement::saveFormControlState() const
         if (!option->selected())
             continue;
         state.append(option->value());
+        state.append(String::number(i));
         if (!multiple())
             break;
     }
@@ -1058,6 +1059,12 @@ size_t HTMLSelectElement::searchOptionsForValue(const String& value, size_t list
     return kNotFound;
 }
 
+String HTMLSelectElement::valueAtIndex(size_t index) const
+{
+    const WillBeHeapVector<RawPtrWillBeMember<HTMLElement> >& items = listItems();
+    return index < items.size() && isHTMLOptionElement(items[index]) ? toHTMLOptionElement(items[index])->value() : WTF::emptyString();
+}
+
 void HTMLSelectElement::restoreFormControlState(const FormControlState& state)
 {
     recalcListItems();
@@ -1073,21 +1080,34 @@ void HTMLSelectElement::restoreFormControlState(const FormControlState& state)
         toHTMLOptionElement(items[i])->setSelectedState(false);
     }
 
+    // The saved state should have at least one value and an index.
+    ASSERT(state.valueSize() >= 2);
     if (!multiple()) {
-        size_t foundIndex = searchOptionsForValue(state[0], 0, itemsSize);
-        if (foundIndex != kNotFound)
-            toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+        size_t index = state[1].toUInt();
+        if (isHTMLOptionElement(items[index]) && valueAtIndex(index) == state[0]) {

[tkent, 2014/09/09 23:55:23]

This line has out-of-bound-access vulnerability.  Please assume |index| is a
tainted value.

[spartha, 2014/09/10 06:33:33]

Done. Sorry that I missed it.

+            toHTMLOptionElement(items[index])->setSelectedState(true);
+        } else {
+            size_t foundIndex = searchOptionsForValue(state[0], 0, itemsSize);
+            if (foundIndex != kNotFound)
+                toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+        }
     } else {
         size_t startIndex = 0;
-        for (size_t i = 0; i < state.valueSize(); ++i) {
+        for (size_t i = 0; i < state.valueSize(); i+= 2) {
             const String& value = state[i];
-            size_t foundIndex = searchOptionsForValue(value, startIndex, itemsSize);
-            if (foundIndex == kNotFound)
-                foundIndex = searchOptionsForValue(value, 0, startIndex);
-            if (foundIndex == kNotFound)
-                continue;
-            toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
-            startIndex = foundIndex + 1;
+            const size_t index = state[i + 1].toUInt();
+            if (isHTMLOptionElement(items[index]) && valueAtIndex(index) == value) {

[tkent, 2014/09/09 23:55:23]

Ditto.

[spartha, 2014/09/10 06:33:33]

Done.

+                toHTMLOptionElement(items[index])->setSelectedState(true);
+                startIndex = index + 1;
+            } else {
+                size_t foundIndex = searchOptionsForValue(value, startIndex, itemsSize);
+                if (foundIndex == kNotFound)
+                    foundIndex = searchOptionsForValue(value, 0, startIndex);
+                if (foundIndex == kNotFound)
+                    continue;
+                toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+                startIndex = foundIndex + 1;
+            }
         }
     }