New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Owner:
Closed: Nov 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Stack-buffer-overflow in base::files::(anonymous namespace)::InotifyReaderTask::Run
Project Member Reported by infe...@chromium.org, Nov 22 2011 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=749155

Fuzzer: Marty_html_twiddler

Crash Type: Stack-buffer-overflow READ 8
Crash Address: 0x7f764f5ae8a0
Crash State:
  - crash stack -
  base::files::(anonymous namespace)::InotifyReaderTask::Run
  base::subtle::TaskClosureAdapter::Run
  MessageLoop::RunTask
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv97das2dC0v0yi-6q4KtmQX0T5BmpLIF09QOEwJGO0dkNhN-FykK-WnFsvO5ur4sANYgGvhWCeU8L5phZ77hVFE6ZrarYyCAv6SwFI66dVgBH2sniWOfGt0Inng4tVILkJbV55NiWys6vqhF6lbj6CcoKeHRIw
 
Cc: dmaclach@chromium.org mnissler@chromium.org
Labels: OS-Linux
This can be SecCritical, but i suspect the attack vector depends on the command line params ?

mnissler@, dmaclach@, can you please take a look.
Kostya: ah, it actually makes perfect sense now
these FD_ folks are macros
and they access the local variable as an array using the first parameter as an index (sort of)
Abhishek: DCHECK(i + event_size <= static_cast<size_t>(bytes_read));
i dont like DCHECKs
Kostya: hardly related
the bug is on 'rfds' local var
As Kostya triaged this, the bug seems to be probably on one of the FD_ lines.
FD_ISSET(shutdown_fd_, &rfds) is read from local
whereas FD_SET does read-then-write

An fd_set is a fixed size buffer. Executing FD_CLR() or FD_SET() with a value of fd that is negative or is equal to or larger than
FD_SETSIZE will result in undefined behavior. Moreover, POSIX requires fd to be a valid file descriptor.
(from man FD_SET)

We should add a CHECK(inotify_fd_ < FD_SETSIZE)
also check for >= 0. Also do it for shutdown_fd_

Better to crash on a CHECK that a stack buffer overflow.
Note that inotify_fd_ is checked for >= 0 before constructing InotifyReaderTask and shutdown_fd_ comes directly from a pipe() call, so the only fd_set related stack overflow I can see would be due to large file descriptor numbers. FD_SETSIZE is 1024 according to my system headers, so this seems quite unlikely...

Regarding the DCHECK(i + event_size <= static_cast<size_t>(bytes_read)), I don't think there's a possible stack overflow here, assuming that std::vector always allocates its buffer on the heap.

Put up a CL that adds the file descriptor CHECKS: http://codereview.chromium.org/8681006/
Cc: -mnissler@chromium.org
Owner: mnissler@chromium.org
Status: Assigned
Labels: -SecImpacts-None SecImpacts-Stable Merge-Approved SecImpacts-Beta Mstone-15
Status: FixUnreleased
http://src.chromium.org/viewvc/chrome?view=rev&revision=111369
Labels: -Merge-Approved Merge-Merged
Merged to 15 in r111505.
Merged to 16 in r111506.
Labels: -Mstone-15 Mstone-16
Thanks a lot Mattias for the fix and the merge.
Labels: -SecSeverity-High SecSeverity-Medium
Seems like a medium severity. Couldn't get a reliable repro and we don't know to what extent this can be intentionally triggered.
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Cc: g.iucul...@gmail.com
Comment 11 by cdn@chromium.org, May 15 2012
Status: Fixed
Marking old security bugs Fixed..
Project Member Comment 12 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 13 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-Internals -Type-Security -SecSeverity-Medium -SecImpacts-Stable -Stability-AddressSanitizer -SecImpacts-Beta -Mstone-16 Security-Impact-Beta Security-Severity-Medium Cr-Internals Performance-Memory-AddressSanitizer Security-Impact-Stable M-16 Type-Bug-Security
Project Member Comment 14 by bugdroid1@chromium.org, Mar 13 2013
Labels: Restrict-View-EditIssue
Project Member Comment 15 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 17 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 18 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-Medium Security_Severity-Medium
Project Member Comment 19 by bugdroid1@chromium.org, Apr 1 2013
Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer
Project Member Comment 20 by sheriffbot@chromium.org, Jun 14 2016
Labels: -security_impact-beta
Project Member Comment 21 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 22 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
Sign in to add a comment