Google Chrome PasswordFormManager::OnGeneratedPasswordAccepted Heap Buffer Overflow

February 14, 2020   |   by Zeroday
VULNERABILITY DETAILS
```
void PasswordFormManager::OnGeneratedPasswordAccepted(
    FormData form_data,
    uint32_t generation_element_id,
    const base::string16& password) {
  // Find the generating element to update its value. The parser needs a non
  // empty value.
  auto it = std::find_if(form_data.fields.begin(), form_data.fields.end(),
                         [generation_element_id](const auto& field_data) {
                           return generation_element_id ==
                                  field_data.unique_renderer_id;
                         });
  DCHECK(it != form_data.fields.end());
  it->value = password;
[...]
```

`OnGeneratedPasswordAccepted` performs a lookup in the form field vector and then writes to the
entry referenced by the result iterator without first checking whether the iterator is valid. Since
both `form_data.fields` and `generation_element_id` come from a renderer, and the browser process
doesn't perform any additional validation, a compromised renderer can set `generation_element_id` to
a value that the vector doesn't contain, which will result in accessing data past the end of the
vector.

The issue looks exploitable as the code above treats OOB data as an instance of the `FormFieldData`
class and tries to overwrite one of its `std::string` fields while both the vector size and the new
string value are attacker-controlled.

=====================

On further inspection, I've discovered that the initial renderer compromise is not required to
trigger this issue. A malicious web page can force the renderer to construct a field vector that
doesn't contain the ID of the password field. The relevant code snippets are as follows:

```
void PasswordGenerationAgent::FoundFormEligibleForGeneration(
    const PasswordFormGenerationData& form) {
  generation_enabled_fields_[form.new_password_renderer_id] = form; // ***1***

[...]

}

[...]

bool PasswordGenerationAgent::FocusedNodeHasChanged(
    const blink::WebNode& node) {
  if (node.IsNull() || !node.IsElementNode()) {
    return false;
  }

  const blink::WebElement web_element = node.ToConst<blink::WebElement>();
  if (!web_element.GetDocument().GetFrame()) {
    return false;
  }

  const WebInputElement* element = ToWebInputElement(&web_element);
  if (!element)
    return false;

  if (element->IsPasswordFieldForAutofill())
    last_focused_password_element_ = *element;

  auto it =
      generation_enabled_fields_.find(element->UniqueRendererFormControlId());
  if (it != generation_enabled_fields_.end()) { // ***2***
    MaybeCreateCurrentGenerationItem(
        *element, it->second.confirmation_password_renderer_id);
  }
  if (!current_generation_item_ ||
      *element != current_generation_item_->generation_element_) {
    return false;
  }

[...]

  if (!element->IsReadOnly() && element->IsEnabled() &&
      element->Value().length() <= kMaximumCharsForGenerationOffer) {
    MaybeOfferAutomaticGeneration();
    return true;
  }

  return false;
}

[...]

bool ExtractFieldsFromControlElements(
    const WebVector<WebFormControlElement>& control_elements,
    const FieldDataManager* field_data_manager,
    ExtractMask extract_mask,
    std::vector<std::unique_ptr<FormFieldData>>* form_fields,
    std::vector<bool>* fields_extracted,
    std::map<WebFormControlElement, FormFieldData*>* element_map) {
  DCHECK(form_fields->empty());
  DCHECK(element_map->empty());
  DCHECK_EQ(control_elements.size(), fields_extracted->size());

  for (size_t i = 0; i < control_elements.size(); ++i) {
    const WebFormControlElement& control_element = control_elements[i];

    if (!IsAutofillableElement(control_element)) // ***3***
      continue;

    // Create a new FormFieldData, fill it out and map it to the field's name.
    auto form_field = std::make_unique<FormFieldData>();
    WebFormControlElementToFormField(control_element, field_data_manager,
                                     extract_mask, form_field.get());
    (*element_map)[control_element] = form_field.get();
    form_fields->push_back(std::move(form_field));
    (*fields_extracted)[i] = true;

    // To avoid overly expensive computation, we impose a maximum number of
    // allowable fields.
    if (form_fields->size() > kMaxParseableFields)
      return false;
  }

  // Succeeded if fields were extracted.
  return !form_fields->empty();
}
```

When the browser discovers an element that's eligible for password generation, it inserts the
element's ID to the `generation_enabled_fields_` list[1]. Then, if an element in the page gets
focused, `FocusedNodeHasChanged` checks whether its ID is in the eligible element list[2] - and if
it is, sends a password suggestion request to the browser process. The function that constructs the
form field vector, which is a part of the request data, performs its own set of checks to determine
which elements should be skipped[3].

The problem is that the element might be changed between [1] and [2] in a way that will cause the
check in [3] fail. For example, if the element is an <input>, its type can be set to one that's not
supported by the generator. This won't affect the renderer ID, which is only set up in the element
constructor, and won't invalidate `generation_enabled_fields_`. Provided that the page contains at
least one more eligible element to make `ExtractFieldsFromControlElements` succeed, the renderer
will send a request with invalid data.

=====================

The biggest constraint of this vulnerability is that a user has to accept the password suggestion to
trigger it even if the renderer is already compromised. However, as it's possible to make the
suggestion pop-up follow the mouse pointer using JavaScript, stealing a user's click shouldn't be
tough (see repro_cj.html). I'm also confident this approach can be applied to create a much more
convincing test case. 

=====================

I believe the immediate fix for this bug should transform the DCHECK in
`OnGeneratedPasswordAccepted` into a CHECK. It's probably a good idea to also do this for every
other `std::find` or `std::find_if` call in the browser process.


VERSION
Google Chrome 78.0.3904.97 (Official Build) (64-bit)
Chromium 80.0.3972.0 (Developer Build) (64-bit)


REPRODUCTION CASE
Please note that in order to enable the password generation feature you need to turn on Chrome Sync.

patch.diff - the renderer-side patch that simulates the compromised renderer state.
repro_for_patched_renderer.html - essentially just an <input> that trigger the password generator.
repro.html - a test-case that works from a regular renderer.
repro_cj.html - a clickjacking based test-case that makes the password suggestion box follow the
mouse pointer.


CREDIT INFORMATION
Sergei Glazunov of Google Project Zero

Leave Your Comment

3 × 1 =