New issue
Advanced search Search tips

Issue 728983 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Use-of-uninitialized-value in ui::XVisualManager::XVisualManager

Project Member Reported by ClusterFuzz, Jun 2 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5432070846545920

Fuzzer: libFuzzer_gpu_angle_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  ui::XVisualManager::XVisualManager
  base::DefaultSingletonTraits<ui::XVisualManager>::New
  base::Singleton<ui::XVisualManager, base::DefaultSingletonTraits<ui::XVisualMana
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=476541:476588

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5432070846545920


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Jun 2 2017

Labels: M-60
Project Member

Comment 2 by sheriffbot@chromium.org, Jun 2 2017

Labels: ReleaseBlock-Beta
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

Comment 3 by sheriffbot@chromium.org, Jun 2 2017

Labels: Pri-1
Components: Internals>GPU
Owner: piman@chromium.org
Status: Assigned (was: Untriaged)
piman: Would you mind helping with the triage on this one? I'm having trouble figuring out what might be the cause.
Project Member

Comment 5 by sheriffbot@chromium.org, Jun 6 2017

Labels: -Security_Impact-Head Security_Impact-Beta

Comment 6 by piman@chromium.org, Jun 6 2017

Cc: thomasanderson@chromium.org geoffl...@chromium.org
Components: -Internals>GPU Internals>GPU>Internals
I'll take a look. Not clear either - I mildly suspect Xlib is returning partially initialized data that we then use as a key in the hash table.

If legit, in production this would happen at initialization time in the GPU process, before it's exposed to untrusted data, so I don't think it would be a  security issue.
Isn't this the same as  bug 725884 ?

Comment 8 by piman@chromium.org, Jun 6 2017

Well, it does look identical, but the report still shows up after your fix.
Wait nvm, the stack traces are different
Looks like the issue is in std::unordered_map::operator[].

The regression range includes this buildtools roll:
https://chromium.googlesource.com/chromium/src/+/e8b52c26834fbeba662c6cf0073d88aefb7907a8
which includes this libc++ roll:
https://chromium.googlesource.com/chromium/buildtools/+/104574186c17cd4701857454feba8872e52a7d82
...which has these commits:
https://chromium.googlesource.com/chromium/llvm-project/libcxx/+log/57c4059..3a07dd7
Cc: thakis@chromium.org
@#10: good point, I'll take a look if reverting the buildtools roll helps. I'm a bit surprised it would show up here specifically of all places (or perhaps we see a spike of UMRs in unordered_map all over the code base?)
@#11: that change looks like it only fixes a test, so I'm not sure this would be it.
Labels: -M-60 M-61
I can't repro locally, under msan + instrumented libs, event at the last tested (failing) rev (477151).
It is possible that this only repros under the bot configuration (namely the visuals).

Note: CF actually confirms the uninitialized value comes from Xlib:
	==20543==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x2474351 in __emplace_unique_key_args<unsigned long, const std::__1::piecewise_construct_t &, std::__1::tuple<const unsigned long &>, std::__1::tuple<> > buildtools/third_party/libc++/trunk/include/__hash_table:2032:31
#1 0x2474351 in std::__1::unordered_map<unsigned long, std::__1::unique_ptr<ui::XVisualManager::XVisualData, std::__1::default_delete<ui::XVisualManager::XVisualData> >, std::__1::hash<unsigned long>, std::__1::equal_to<unsigned long>, std::__1::allocator<std::__1::pair<unsigned long const, std::__1::unique_ptr<ui::XVisualManager::XVisualData, std::__1::default_delete<ui::XVisualManager::XVisualData> > > > >::operator[](unsigned long const&) buildtools/third_party/libc++/trunk/include/unordered_map:1386
#2 0x2471de0 in ui::XVisualManager::XVisualManager() ui/base/x/x11_util.cc:1392:5
#3 0x2475dca in base::DefaultSingletonTraits<ui::XVisualManager>::New() base/memory/singleton.h:55:16
#4 0x24717df in base::Singleton<ui::XVisualManager, base::DefaultSingletonTraits<ui::XVisualManager>, ui::XVisualManager>::get() base/memory/singleton.h:260:22
#5 0x812278 in gl::(anonymous namespace)::ChooseConfig(gl::GLSurfaceFormat, bool) ui/gl/gl_surface_egl.cc:295:5
#6 0x811729 in gl::GLSurfaceEGL::GetConfig() ui/gl/gl_surface_egl.cc:496:15
#7 0x81906d in gl::PbufferGLSurfaceEGL::Initialize(gl::GLSurfaceFormat) ui/gl/gl_surface_egl.cc:1076:40
#8 0x7fc7e2 in gl::GLSurface::Initialize() ui/gl/gl_surface.cc:32:10
#9 0x49bbe5 in gpu::(anonymous namespace)::CommandBufferSetup::CommandBufferSetup() gpu/command_buffer/tests/fuzzer_main.cc:116:15
#10 0x4267f1 in __cxx_global_var_init gpu/command_buffer/tests/fuzzer_main.cc:285:35
#11 0x28e41ac in __libc_csu_init
#12 0x7f74fe86fed4 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:246
#13 0x42d872 in _start
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was stored to memory at
#0 0x453622 in __interceptor_realloc
#1 0x7f74fd259fb3 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:134
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101
Uninitialized value was created by a heap allocation
#0 0x45377d in __interceptor_malloc
#1 0x7f74fd259db4 in XGetVisualInfo /build/buildd/libx11-1.6.2/src/VisUtil.c:78
#2 0x5f0fc6 in std::__1::__compressed_pair_elem<std::__1::allocator<int>&, 1, false>::__get() buildtools/third_party/libc++/trunk/include/memory:2101

This matches the code in Xlib v1.6.2 (Xmalloc on l.78, Xrealloc on l.134).
That being said, I fail to understand how that would happen. Xrealloc gets called if count was incremented (10 more since last round), and count is only incremented after the value is written.
Project Member

Comment 15 by ClusterFuzz, Jun 7 2017

ClusterFuzz has detected this issue as fixed in range 477433:477527.

Detailed report: https://clusterfuzz.com/testcase?key=5432070846545920

Fuzzer: libFuzzer_gpu_angle_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  ui::XVisualManager::XVisualManager
  base::DefaultSingletonTraits<ui::XVisualManager>::New
  base::Singleton<ui::XVisualManager, base::DefaultSingletonTraits<ui::XVisualMana
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=476541:476588
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=477433:477527

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5432070846545920


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 16 by ClusterFuzz, Jun 7 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5432070846545920 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 17 by sheriffbot@chromium.org, Jun 7 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Looks like this is happy now, but thomasanderson: https://chromium.googlesource.com/chromium/llvm-project/libcxx/+/bcb4a4e5d76f7da953c96b7d254ca3511156bac6 is a test-only change and should affect our code. So possibly fixed due to a different libc++ upstream change?
Labels: -ReleaseBlock-Beta
Project Member

Comment 20 by sheriffbot@chromium.org, Sep 13 2017

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