New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 169972 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security
ext



Sign in to add a comment

Security: Heap-Buffer-Overflow in usb_api.cc:CreateBufferForTransfer

Project Member Reported by mea...@chromium.org, Jan 15 2013

Issue description

CreateBufferForTransfer in usb_api.cc has the following code snippet:
...
  if (!input.data.get())
    return buffer;

  memcpy(buffer->data(), input.data->data(), size);
  return buffer;
...

Both input.data and size are controlled via JS. When input.data is small enough, an invalid read happens at memcpy:

      chrome.usb.bulkTransfer(device, {
        "direction":"in",
        "endpoint":0,
        "length":100, // size
        "data":new ArrayBuffer(1) // input.data
        }, function() {}
      );

The length of the invalid read can be controlled by changing |length| and the size of |data|.

chrome.usb API documentation implies that |length| and |data| are mutually exclusive for GenericTransfer type: If "direction"=="in", "data" is ignored; and if "direction"=="out", "length" is ignored. The logic in CreateBufferForTransfer still attempts to use "data" when "direction"=="in".
The same logic applies to other usb API that uses CreateBufferForTransfer, including chrome.usb.controlTransfer, interruptTransfer etc.

Attached app is the reproduction case.

Tested with ASAN on Mac and Linux but should also affect Windows .


Crash State:
==35711== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x48871fb3 at pc 0x3884b41 bp 0xb4aa54b8 sp 0xb4aa54b4
READ of size 1 at 0x48871fb3 thread T17
    #0 0x3884b40 in scoped_refptr<net::IOBuffer> ::CreateBufferForTransfer<extensions::api::usb::GenericTransferInfo> usb_api.cc:160
    #1 0x3883ddb in extensions::UsbBulkTransferFunction::AsyncWorkStart usb_api.cc:521
    #2 0x35ee840 in extensions::AsyncApiFunction::WorkOnWorkThread api_function.cc:71

0x48871fb3 is located 13 bytes to the left of 32-byte region [0x48871fc0,0x48871fe0)
freed by thread T0 here:
    #0 0x109845 in __asan_unpoison_memory_region (in Chromium) + 117
    #1 0x97555327 in CFAllocatorDeallocate (in CoreFoundation) + 231
    #2 0x97555089 in CFRelease (in CoreFoundation) + 2041

 
knob-CreateBufferForTransfer.zip
7.3 KB Download
Cc: -gdk@chromium.org
Owner: gdk@chromium.org
Status: Assigned

Comment 2 by mea...@chromium.org, Jan 18 2013

I have a patch for this:

https://codereview.chromium.org/11926005/
Cc: gdk@chromium.org miket@chromium.org
Owner: mea...@chromium.org
Status: Started
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 20 2013

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=177857

------------------------------------------------------------------------
r177857 | meacer@chromium.org | 2013-01-20T00:38:15.984588Z

Changed paths:
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/extensions/api_test/usb/invalid_length_transfer/test.js?r1=177857&r2=177856&pathrev=177857
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/extensions/api_test/usb/invalid_length_transfer/manifest.json?r1=177857&r2=177856&pathrev=177857
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/extensions/api_test/usb/invalid_length_transfer?r1=177857&r2=177856&pathrev=177857
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/api/usb/usb_api.cc?r1=177857&r2=177856&pathrev=177857
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/api/usb/usb_apitest.cc?r1=177857&r2=177856&pathrev=177857

Fix transfer length validation in Usb Api.

This change limits the length parameter of Usb transfers to 0-100 MB.
It introduces a new error for invalid transfer lengths.
It also correctly ignores 'data' parameter if transfer 'direction'
equals 'in', and ignores transfer 'length' parameter if 'direction'
is 'out'.

BUG= 169972 ,  169981 
TEST=browser_tests: UsbApiTest.InvalidLengthTransfer
TBR=gdk@chromium.org

Review URL: https://chromiumcodereview.appspot.com/11926005
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: Fixed
Labels: -Mstone-24 -Merge-Approved Mstone-26 Release-0
M26 seems fine?
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Security -Area-Internals -SecSeverity-Medium -SecImpacts-Stable -Mstone-26 -Feature-Apps-APIs Cr-Platform-Apps-API M-26 Security-Severity-Medium Cr-Internals Security-Impact-Stable Type-Bug-Security
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-Medium Security_Severity-Medium
Labels: CVE-2013-0923
Labels: -Restrict-View-SecurityNotify
Bulk release of old security bug reports.

Project Member

Comment 12 by sheriffbot@chromium.org, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 by sheriffbot@chromium.org, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment