Skip to content

Commit

Permalink
Fix memory read overflow on a local variable
Browse files Browse the repository at this point in the history
The function 'CreateFrame' is copying 2 objects of type RawBattOrSample.
But, there is only one instance in frame_header (line 794).

This issue was found with llvm ASAN.

=================================================================
==17892==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x002eeb34 at pc 0x00fd0559 bp 0x002eea84 sp 0x002eea
74
READ of size 8 at 0x002eeb34 thread T0
==17892==*** WARNING: Failed to initialize DbgHelp!              ***
==17892==*** Most likely this means that the app is already      ***
==17892==*** using DbgHelp, possibly with incompatible flags.    ***
==17892==*** Due to technical reasons, symbolization might crash ***
==17892==*** or produce wrong results.                           ***
    #0 0xfd0573 in __asan_memcpy d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:413
    chromium#1 0xafdf7f in battor::`anonymous namespace'::CreateFrame+0x132 (D:\src\chromium\src\out\ninja\battor_agent_unittest
s.exe+0x40df7f)
    chromium#2 0xb0a92a in battor::BattOrAgentTest_StopTracingFailsIfDataFrameMissingByte_Test::TestBody D:\src\chromium\src\too
ls\battor_agent\battor_agent_unittest.cc:799
    chromium#3 0xd302e3 in testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>+0xec (D:\src\chromium\src\
out\ninja\battor_agent_unittests.exe+0x6402e3)
    chromium#4 0xd2fec9 in testing::Test::Run D:\src\chromium\src\testing\gtest\src\gtest.cc:2474
    chromium#5 0xd318e3 in testing::TestInfo::Run D:\src\chromium\src\testing\gtest\src\gtest.cc:2656
    chromium#6 0xd3296c in testing::TestCase::Run D:\src\chromium\src\testing\gtest\src\gtest.cc:2774
    chromium#7 0xd43548 in testing::internal::UnitTestImpl::RunAllTests+0x80a (D:\src\chromium\src\out\ninja\battor_agent_unitte
sts.exe+0x653548)
    chromium#8 0xd42cac in testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>+0xec (D:
\src\chromium\src\out\ninja\battor_agent_unittests.exe+0x652cac)
    chromium#9 0xd429b2 in testing::UnitTest::Run+0x1c8 (D:\src\chromium\src\out\ninja\battor_agent_unittests.exe+0x6529b2)
    chromium#10 0xddab3d in base::TestSuite::Run D:\src\chromium\src\base\test\test_suite.cc:266
    chromium#11 0xddc695 in base::`anonymous namespace'::LaunchUnitTestsInternal D:\src\chromium\src\base\test\launcher\unit_tes
t_launcher.cc:209
    chromium#12 0xddc3a6 in base::LaunchUnitTests D:\src\chromium\src\base\test\launcher\unit_test_launcher.cc:453
    chromium#13 0xdda7b9 in main D:\src\chromium\src\base\test\run_all_unittests.cc:12
    chromium#14 0xfefdf8 in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253
    chromium#15 0x749b3369 in BaseThreadInitThunk+0x11 (C:\Windows\syswow64\kernel32.dll+0x7dd73369)
    chromium#16 0x76f49901 in RtlInitializeExceptionChain+0x62 (C:\Windows\SysWOW64\ntdll.dll+0x7dea9901)
    chromium#17 0x76f498d4 in RtlInitializeExceptionChain+0x35 (C:\Windows\SysWOW64\ntdll.dll+0x7dea98d4)

Address 0x002eeb34 is located in stack of thread T0 at offset 116 in frame
    #0 0xb0a73f in battor::BattOrAgentTest_StopTracingFailsIfDataFrameMissingByte_Test::TestBody D:\src\chromium\src\too
ls\battor_agent\battor_agent_unittest.cc:784

  This frame has 16 object(s):
    [16, 22) 'cal_frame_header'
    [48, 52) 'cal_frame'
    [64, 68) 'ref.tmp'
    [80, 86) 'frame_header'
    [112, 116) 'frame' <== Memory access at offset 116 overflows this variable
    [128, 132) 'frame_bytes'
    [144, 148) 'ref.tmp2'
    [160, 168) 'gtest_ar_'
    [192, 196) 'ref.tmp12'
    [208, 212) 'temp.lvalue'
    [224, 248) 'ref.tmp14'
    [288, 296) 'gtest_ar'
    [320, 324) 'ref.tmp17'
    [336, 340) 'ref.tmp19'
    [352, 356) 'ref.tmp21'
    [368, 372) 'temp.lvalue23'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow d:\src\llvm\llvm\projects\compiler-rt\lib\asan\asan_interceptors.cc:413
 in __asan_memcpy
Shadow bytes around the buggy address:
  0x3005dd10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3005dd20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3005dd30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3005dd40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3005dd50: 00 00 00 00 00 00 00 00 f1 f1 06 f2 f2 f2 04 f2
=>0x3005dd60: 04 f2 06 f2 f2 f2[04]f2 04 f2 04 f2 00 f2 f2 f2
  0x3005dd70: 04 f2 04 f2 00 00 00 f2 f2 f2 f2 f2 00 f2 f2 f2
  0x3005dd80: 04 f2 04 f2 04 f2 04 f3 00 00 00 00 00 00 00 00
  0x3005dd90: 00 00 00 00 00 00 00 00 f1 f1 04 f3 00 00 00 00
  0x3005dda0: 00 00 00 00 00 00 00 00 f1 f1 04 f2 04 f2 04 f3
  0x3005ddb0: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==17892==ABORTING
[1/1] BattOrAgentTest.StopTracingFailsIfDataFrameMissingByte (CRASHED)
1 test crashed:
    BattOrAgentTest.StopTracingFailsIfDataFrameMissingByte (../../tools/battor_agent/battor_agent_unittest.cc:784)
Tests took 0 seconds.

R=nednguyen@google.com, chrisha@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2575993002
Cr-Commit-Position: refs/heads/master@{#442594}
  • Loading branch information
bergeret authored and Commit bot committed Jan 10, 2017
1 parent 432cb34 commit df8ca76
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion tools/battor_agent/battor_agent_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ TEST_F(BattOrAgentTest, StopTracingFailsIfDataFrameMissingByte) {

// Remove the last byte from the frame to make it invalid.
std::unique_ptr<vector<char>> frame_bytes =
CreateFrame(frame_header, frame, 2);
CreateFrame(frame_header, frame, 1);
frame_bytes->pop_back();

OnMessageRead(true, BATTOR_MESSAGE_TYPE_SAMPLES, std::move(frame_bytes));
Expand Down

0 comments on commit df8ca76

Please sign in to comment.