New issue
Advanced search Search tips

Issue 841571 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

trunks: ASan issues in unit tests

Project Member Reported by emaxx@chromium.org, May 9 2018

Issue description

Running unit tests under USE="asan" revealed these issues:

  * ASAN error detected:
  * =================================================================
  * ==17==ERROR: AddressSanitizer: global-buffer-overflow on address 0x560bd4da8c70 at pc 0x560bd3ae36a6 bp 0x7fff37d50bd0 sp 0x7fff37d50380
  * READ of size 100 at 0x560bd4da8c70 thread T0
  *     #0 0x560bd3ae36a5 in __asan_memcpy (/var/cache/portage/chromeos-base/trunks/out/Default/trunks_testrunner+0x1356a5)
  *     #1 0x560bd3e7d0f9 in memcpy(void*, void const* pass_object_size0, unsigned long) /build/snappy/var/cache/portage/chromeos-base/trunks/out/Default/../../../../../../../usr/include/bits/string3.h:65:10
  *     #2 0x560bd3e7d0f9 in std::__1::char_traits<char>::copy(char*, char const*, unsigned long) /usr/bin/../include/c++/v1/__string:223
  *     #3 0x560bd3e7d0f9 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__init(char const*, unsigned long) /usr/bin/../include/c++/v1/string:1567
  *     #4 0x560bd3e7d0f9 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::basic_string(char const*, unsigned long) /usr/bin/../include/c++/v1/string:1599
  *     #5 0x560bd3e7d0f9 in trunks::TpmUtilityTest_StirRandomSuccess_Test::TestBody() /build/snappy/var/cache/portage/chromeos-base/trunks/out/Default/../../../../../../../../../mnt/host/source/src/platform2/trunks/tpm_utility_test.cc:409
  *     #6 0x7f3015a607d3 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/usr/lib64/libgtest.so.0+0x467d3)
  *     #7 0x7f3015a44320 in testing::Test::Run() (/usr/lib64/libgtest.so.0+0x2a320)
  *     #8 0x7f3015a4547f in testing::TestInfo::Run() (/usr/lib64/libgtest.so.0+0x2b47f)
  *     #9 0x7f3015a45b76 in testing::TestCase::Run() (/usr/lib64/libgtest.so.0+0x2bb76)
  *     #10 0x7f3015a4eb66 in testing::internal::UnitTestImpl::RunAllTests() (/usr/lib64/libgtest.so.0+0x34b66)
  *     #11 0x7f3015a615a3 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/usr/lib64/libgtest.so.0+0x475a3)
  *     #12 0x7f3015a4e71e in testing::UnitTest::Run() (/usr/lib64/libgtest.so.0+0x3471e)
  *     #13 0x560bd4053efb in RUN_ALL_TESTS() /build/snappy/var/cache/portage/chromeos-base/trunks/out/Default/../../../../../../../usr/include/gtest/gtest.h:2233:46
  *     #14 0x560bd4053efb in main /build/snappy/var/cache/portage/chromeos-base/trunks/out/Default/../../../../../../../../../mnt/host/source/src/platform2/trunks/trunks_testrunner.cc:30
  *     #15 0x7f3013d3a735 in __libc_start_main /var/tmp/portage/cross-x86_64-cros-linux-gnu/glibc-2.23-r18/work/glibc-2.23/csu/../csu/libc-start.c:289
  *     #16 0x560bd3a3db48 in _start (/var/cache/portage/chromeos-base/trunks/out/Default/trunks_testrunner+0x8fb48)
  * 
  * 0x560bd4da8c70 is located 48 bytes to the left of global variable '<string literal>' defined in '../../../../../../../../../mnt/host/source/src/platform2/trunks/tpm_utility_test.cc:410:3' (0x560bd4da8ca0) of size 65
  *   '<string literal>' is ascii string 'utility_.StirRandom(entropy_data, &mock_authorization_delegate_)'
  * 0x560bd4da8c70 is located 0 bytes to the right of global variable '<string literal>' defined in '../../../../../../../../../mnt/host/source/src/platform2/trunks/tpm_utility_test.cc:409:28' (0x560bd4da8c60) of size 16
  *   '<string literal>' is ascii string 'large test data'
  * SUMMARY: AddressSanitizer: global-buffer-overflow (/var/cache/portage/chromeos-base/trunks/out/Default/trunks_testrunner+0x1356a5) in __asan_memcpy
  * Shadow bytes around the buggy address:
  *   0x0ac1fa9ad130: 02 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  *   0x0ac1fa9ad140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  *   0x0ac1fa9ad150: 00 00 00 00 00 00 00 00 00 00 00 00 04 f9 f9 f9
  *   0x0ac1fa9ad160: f9 f9 f9 f9 00 00 00 00 00 00 01 f9 f9 f9 f9 f9
  *   0x0ac1fa9ad170: 00 00 02 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  * =>0x0ac1fa9ad180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9
  *   0x0ac1fa9ad190: f9 f9 f9 f9 00 00 00 00 00 00 00 00 01 f9 f9 f9
  *   0x0ac1fa9ad1a0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  *   0x0ac1fa9ad1b0: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00
  *   0x0ac1fa9ad1c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  *   0x0ac1fa9ad1d0: 00 02 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 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
  * ==17==ABORTING

 

Comment 1 by emaxx@chromium.org, May 9 2018

Owner: emaxx@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, May 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8fcfb33e98a6680d65db4f75eb97c4a6983979cb

commit 8fcfb33e98a6680d65db4f75eb97c4a6983979cb
Author: Maksim Ivanov <emaxx@google.com>
Date: Sat May 12 14:19:59 2018

trunks: Fix buf-overflow and memleak in unittests

* SessionManagerTest was leaking HmacAuthorizationDelegate;
* TpmUtilityTest was accessing std::string beyond the string end;

BUG= chromium:841571 
TEST=running unit tests under USE="asan"

Change-Id: I3218c9a438357aae1a916d25aa306eee67ea4be1
Reviewed-on: https://chromium-review.googlesource.com/1053188
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/8fcfb33e98a6680d65db4f75eb97c4a6983979cb/trunks/session_manager_test.cc
[modify] https://crrev.com/8fcfb33e98a6680d65db4f75eb97c4a6983979cb/trunks/tpm_utility_test.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4eb567c35e2f81354fd6f92333d437dddc0dcfab

commit 4eb567c35e2f81354fd6f92333d437dddc0dcfab
Author: Maksim Ivanov <emaxx@google.com>
Date: Thu May 17 10:24:27 2018

trunks: Fix structs initialization in unit tests

Avoid copying uninitialized structs in the
GetPublicRSAEndorsementKeyModulus* unit tests.

Also do some other minor cleanup in these tests.

BUG= chromium:841571 
TEST=existing unit tests

Change-Id: Idb940f76b4501d49161a0abc0dfa1575e5524aab
Reviewed-on: https://chromium-review.googlesource.com/1053448
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/4eb567c35e2f81354fd6f92333d437dddc0dcfab/trunks/tpm_utility_test.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7ee053b2de8360bf11a7bbefd5c7fbb75ab38c65

commit 7ee053b2de8360bf11a7bbefd5c7fbb75ab38c65
Author: Maksim Ivanov <emaxx@google.com>
Date: Fri May 18 17:05:26 2018

trunks: Fix uninitialized read in unit test

This fixes the failure of the
TpmUtilityTest.GetPublicRSAEndorsementKeyModulus_NoDataInNvram
test under ASan.

The failure was caused by the read of uninitialized integer
TPMS_NV_PUBLIC.data_size in
TpmUtilityImpl::GetPublicRSAEndorsementKeyModulus(). Normally
this field is initialized by Tpm::NV_ReadPublicSync(), but the
test's default mock behavior was returning TPM_RC_SUCCESS
without touching the returned structure. The rest of the test,
consequently, falls under undefined behavior - but it was
failing only if the uninitialized integer turns out to be large
enough to bail out from ReadNVSpace() with SAPI_RC_BAD_SIZE
(which was happening under ASan).

The fix is to provide the missing mock, so that the Tpm method
doesn't report success without setting the data.

BUG= chromium:841571 
TEST=Run the GetPublicRSAEndorsementKeyModulus_NoDataInNvram
     test under USE=asan

Change-Id: I7f94fdc768969c4831b8fee9acf537e952d6e754
Reviewed-on: https://chromium-review.googlesource.com/1053447
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/7ee053b2de8360bf11a7bbefd5c7fbb75ab38c65/trunks/tpm_utility_test.cc

Comment 5 by emaxx@chromium.org, May 20 2018

Status: Fixed (was: Started)

Sign in to add a comment