688889 - Investigate stack buffer overflow in UpdateEmail

Status: RESOLVED DUPLICATE of bug 688877

(Toolkit :: Crash Reporting, defect)

Description

[Brandon Sterne (:bsterne)]

Fabian Yamaguchi of Recurity Labs on behalf of BSI (German Federal Office for Information Security) reported the following to [e-mail]:

Function: UpdateEmail

Component:
The bug is contained in the Crash Reporter Implementation for Windows, in function UpdateEmail in particular.

Summary:
The function UpdateEmail reads textual data from a dialog item into a local buffer using the Win32 API function GetDlgItemText. This function expects the number of wide­characters that the destination buffer can hold as its last argument. In UpdateEmail, this third parameter has mistakenly been chosen to contain the number of bytes the buffer consists of, which is twice as much as the number of wide­characters it can hold. Therefore, it may be possible to trigger a stack­based buffer­ overflow.

Actual Results:
A local stack­buffer can potentially be overflowed.

Expected Results:
The buffer cannot be overflowed.

Additional Information:
static void UpdateEmail(HWND hwndDlg)
{
  if (IsDlgButtonChecked(hwndDlg, IDC_EMAILMECHECK)) {
    wchar_t email[MAX_EMAIL_LENGTH];
    GetDlgItemText(hwndDlg, IDC_EMAILTEXT, email, sizeof(email));
    gQueryParameters[L"Email"] = email;
    if (IsDlgButtonChecked(hwndDlg, IDC_SUBMITREPORTCHECK))
      EnableWindow(GetDlgItem(hwndDlg, IDC_EMAILTEXT), true);
  } else {
    gQueryParameters.erase(L"Email");
    EnableWindow(GetDlgItem(hwndDlg, IDC_EMAILTEXT), false);
  }
}

Comment 1

[Daniel Veditz [:dveditz]]

Valid bug but doesn't seem that the user is likely to hack themself by typing in a really really long email address.