New issue
Advanced search Search tips

Issue 744213 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug-Security



Sign in to add a comment

Security: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int' (base/message_loop/incoming_task_queue.cc)

Reported by brian.ca...@gmail.com, Jul 17 2017

Issue description

VULNERABILITY DETAILS
While fuzzing Chromium `73c48a9` with libFuzzer (compiled by following my own guide @ http://www.geeknik.net/9t76jygu1), this `signed integer overflow` bug was triggered with the `net_http_server_fuzzer`. 

Line 171 is the first line in what I believe to be the affected block of code:

    pending_task->sequence_num = next_sequence_num_++;

    message_loop_->task_annotator()->DidQueueTask("MessageLoop::PostTask",
                                                  *pending_task);

    bool was_empty = incoming_queue_.empty();
    incoming_queue_.push(std::move(*pending_task));

    if (is_ready_for_scheduling_ &&
        (always_schedule_work_ || (!message_loop_scheduled_ && was_empty))) {
      schedule_work = true;
      // After we've scheduled the message loop, we do not need to do so again
      // until we know it has processed all of the work in our queue and is
      // waiting for more work again. The message loop will always attempt to
      // reload from the incoming queue before waiting again so we clear this
      // flag in ReloadWorkQueue().
      message_loop_scheduled_ = true;
    }



VERSION
Chrome Version: Git 73c48a9
Operating System: Ubuntu 16.04

REPRODUCTION CASE
Attached

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: undefined-behavior, signed integer overflow

#167717081      NEW    cov: 3519 ft: 7539 corp: 776/3831Kb exec/s: 405 rss: 666Mb L: 7125 MS: 1 CopyPart-
#183518462      NEW    cov: 3519 ft: 7541 corp: 777/3832Kb exec/s: 413 rss: 666Mb L: 1179 MS: 2 ChangeBit-ChangeBit-
#188812817      NEW    cov: 3520 ft: 7542 corp: 778/3833Kb exec/s: 414 rss: 666Mb L: 932 MS: 2 EraseBytes-ChangeASCIIInt-
../../base/message_loop/incoming_task_queue.cc:171:52: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
    #0 0x1326f05 in _ZN4base8internal17IncomingTaskQueue15PostPendingTaskEPNS_11PendingTaskE /root/chromium/src/base/message_loop/incoming_task_queue.cc:171
    #1 0x1326f05 in ?? ??:0
    #2 0x13263ab in _ZN4base8internal17IncomingTaskQueue18AddToIncomingQueueERKN15tracked_objects8LocationENS_8CallbackIFvvELNS0_8CopyModeE0ELNS0_10RepeatModeE0EEENS_9TimeDeltaEb /root/chromium/src/base/message_loop/incoming_task_queue.cc:86
    #3 0x13263ab in ?? ??:0
    #4 0x127117a in _ZN4base8internal21MessageLoopTaskRunner15PostDelayedTaskERKN15tracked_objects8LocationENS_8CallbackIFvvELNS0_8CopyModeE0ELNS0_10RepeatModeE0EEENS_9TimeDeltaE /root/chromium/src/base/message_loop/message_loop_task_runner.cc:32
    #5 0x127117a in ?? ??:0
    #6 0x129ed58 in _ZN4base10TaskRunner8PostTaskERKN15tracked_objects8LocationENS_8CallbackIFvvELNS_8internal8CopyModeE0ELNS7_10RepeatModeE0EEE /root/chromium/src/base/task_runner.cc:47
    #7 0x129ed58 in ?? ??:0
    #8 0x583617 in _ZN3net12FuzzedSocket5WriteEPNS_8IOBufferEiRKN4base8CallbackIFviELNS3_8internal8CopyModeE1ELNS6_10RepeatModeE1EEE /root/chromium/src/net/socket/fuzzed_socket.cc:126
    #9 0x583617 in ?? ??:0
    #10 0x1226009 in _ZN3net10HttpServer11DoWriteLoopEPNS_14HttpConnectionE /root/chromium/src/net/server/http_server.cc:290
    #11 0x1226009 in ?? ??:0
    #12 0x584087 in Run /root/chromium/src/base/callback.h:80
    #13 0x584087 in OnWriteComplete /root/chromium/src/net/socket/fuzzed_socket.cc:268
    #14 0x584087 in ?? ??:0
    #15 0x1312b46 in Run /root/chromium/src/base/callback.h:91
    #16 0x1312b46 in RunTask /root/chromium/src/base/debug/task_annotator.cc:59
    #17 0x1312b46 in ?? ??:0
    #18 0x1264e6e in _ZN4base11MessageLoop7RunTaskEPNS_11PendingTaskE /root/chromium/src/base/message_loop/message_loop.cc:422
    #19 0x1264e6e in ?? ??:0
    #20 0x1266526 in _ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE /root/chromium/src/base/message_loop/message_loop.cc:433
    #21 0x1266526 in ?? ??:0
    #22 0x1267b88 in _ZN4base11MessageLoop6DoWorkEv /root/chromium/src/base/message_loop/message_loop.cc:540
    #23 0x1267b88 in ?? ??:0
    #24 0x127876c in _ZN4base19MessagePumpLibevent3RunEPNS_11MessagePump8DelegateE /root/chromium/src/base/message_loop/message_pump_libevent.cc:219
    #25 0x127876c in ?? ??:0
    #26 0x1282ada in _ZN4base7RunLoop3RunEv /root/chromium/src/base/run_loop.cc:111
    #27 0x1282ada in ?? ??:0
    #28 0x56409a in LLVMFuzzerTestOneInput /root/chromium/src/net/server/http_server_fuzzer.cc:107
    #29 0x56409a in ?? ??:0
    #30 0x5b6581 in _ZN6fuzzer6Fuzzer15ExecuteCallbackEPKhm /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:451
    #31 0x5b6581 in ?? ??:0
    #32 0x5b6d77 in _ZN6fuzzer6Fuzzer6RunOneEPKhm /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:408
    #33 0x5b6d77 in ?? ??:0
    #34 0x5b83b3 in _ZN6fuzzer6Fuzzer16MutateAndTestOneEv /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:587
    #35 0x5b83b3 in ?? ??:0
    #36 0x5b8957 in _ZN6fuzzer6Fuzzer4LoopEv /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:615
    #37 0x5b8957 in ?? ??:0
    #38 0x592a1e in _ZN6fuzzer12FuzzerDriverEPiPPPcPFiPKhmE /root/chromium/src/third_party/libFuzzer/src/FuzzerDriver.cpp:681
    #39 0x592a1e in ?? ??:0
    #40 0x5bfce4 in main /root/chromium/src/third_party/libFuzzer/src/FuzzerMain.cpp:20
    #41 0x5bfce4 in ?? ??:0
    #42 0x7f3c60b8d3f0 in __libc_start_main ??:?
    #43 0x7f3c60b8d3f0 in ?? ??:0

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior base/message_loop/incoming_task_queue.cc:171:52 in
MS: 2 CopyPart-ChangeBit-; base unit: ff4a7fcdec01c707605ea916c3c32c8465214b97
artifact_prefix='./'; Test unit written to ./crash-6b68fd7cfec89c58dcb3ca399ccd54ce8a5b263e
==27339== ERROR: libFuzzer: deadly signal
    #0 0x540567 in __sanitizer_print_stack_trace ??:?
    #1 0x540567 in ?? ??:0
    #2 0x5b35ed in _ZN6fuzzer6Fuzzer13CrashCallbackEv /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:195
    #3 0x5b35ed in ?? ??:0
    #4 0x5b3545 in _ZN6fuzzer6Fuzzer25StaticCrashSignalCallbackEv /root/chromium/src/third_party/libFuzzer/src/FuzzerLoop.cpp:179
    #5 0x5b3545 in ?? ??:0
    #6 0x7f3c628a266f in __funlockfile ??:?
    #7 0x7f3c628a266f in ?? ??:0

NOTE: libFuzzer has rudimentary signal handlers.
      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal
MS: 2 CopyPart-ChangeBit-; base unit: ff4a7fcdec01c707605ea916c3c32c8465214b97
artifact_prefix='./'; Test unit written to ./crash-6b68fd7cfec89c58dcb3ca399ccd54ce8a5b263e
 
crash-6b68fd7cfec89c58dcb3ca399ccd54ce8a5b263e.gz
828 bytes Download

Comment 1 by raymes@chromium.org, Jul 18 2017

Cc: robliao@chromium.org dcheng@chromium.org
Components: Internals>TaskScheduler
Labels: Security_Severity-Medium Security_Impact-Stable
Owner: gab@chromium.org
Status: Assigned (was: Unconfirmed)
This looks like an integer overflow. I'm not sure the implications of that in this code. 

gab: could you please take a look?
Labels: Pri-2
Wow, lots of tasks must have been posted. A quick reading of the code suggests that the sequence number is used for tracing and for ordering delayed tasks (see PendingTask::operator<)

The ordering one could be problematic as it would cause out-of-order task execution within a sequence. However, the likelihood of this occurring is probably small.

My inclination is P2 here.

Comment 3 by raymes@chromium.org, Jul 18 2017

Labels: OS-All
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 18 2017

Labels: M-60
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 18 2017

Labels: -Pri-2 Pri-1

Comment 6 by gab@chromium.org, Jul 18 2017

Cc: gab@chromium.org
Labels: -Pri-1 -Security_Severity-Medium Security_Severity-Low Pri-3
Owner: ----
Status: Available (was: Assigned)
Right, I'm happy to review a CL that makes |next_sequence_num_| a size_t but this has been that way for years and thus not a big issue IMO (and also not a security issue).
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 6 2017

Labels: -M-60 M-61
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 18 2017

Labels: -M-61 M-62
Project Member

Comment 9 by sheriffbot@chromium.org, Dec 7 2017

Labels: -M-62 M-63
Project Member

Comment 10 by sheriffbot@chromium.org, Jan 25 2018

Labels: -M-63 M-64
Project Member

Comment 11 by ClusterFuzz, Feb 14 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5378680390680576.
Project Member

Comment 12 by sheriffbot@chromium.org, Mar 7 2018

Labels: -M-64 M-65
Project Member

Comment 13 by sheriffbot@chromium.org, Apr 19 2018

Labels: -M-65 M-66

Comment 14 by gab@chromium.org, May 3 2018

Status: WontFix (was: Available)
This is WAI. It's a 64bit uint used as a "unique" number for PendingTask::sequence_num. Overflow here is very unlikely and will not even cause runtime issues as the number being reused is for sure no longer used by the previous task it was assigned to (or we have >trillions of tasks queued...)
Project Member

Comment 15 by ClusterFuzz, May 6 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5568217121292288.
Project Member

Comment 16 by sheriffbot@chromium.org, Aug 10

Labels: -Restrict-View-SecurityTeam allpublic
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

Sign in to add a comment