Null-dereference READ in SkDiscardableMemory::Create |
|||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5515976856305664 Fuzzer: libFuzzer_skia_image_decode_fuzzer Job Type: windows_libfuzzer_chrome_asan Platform Id: windows Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: SkDiscardableMemory::Create SkBitmapCache::Alloc SkImage_Lazy::getROPixels Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_libfuzzer_chrome_asan&range=607281:607292 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5515976856305664 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing_on_windows.md for more information.
,
Nov 14
I find the test case size (4B) very suspicious (probably a bug in fuzzer and not tested code). This doesn't repro on OSS-Fuzz/Linux: https://oss-fuzz.com/v2/testcase-detail/5178066481971200?noredirect=1
,
Nov 14
Leon, can you try reproducing this on a windows machine? out/Release/fuzz -t image_decode -b [file] should be enough to reproduce (if the auto-detect doesn't work)
,
Nov 14
This crash occurs very frequently on windows platform and is likely preventing the fuzzer skia_image_decode_fuzzer from making much progress. Fixing this will allow more bugs to be found. Marking this bug as a blocker for next Beta release. If this is incorrect, please add ClusterFuzz-Wrong label and remove the ReleaseBlock-Beta label.
,
Nov 14
Unfortunately, I don't have a Windows machine to test on :(
But I've looked at the file - we interpret it as a 1 x 43 WBMP. All it has is the width and height; there is no following data.
The SkWbmpCodec reports that it has decoded 0 rows, and then SkCodec fills all rows with transparent.
The null dereference is here:
SkDiscardableMemory* SkDiscardableMemory::Create(size_t bytes) {
return new SkDiscardableMemoryChrome(
base::DiscardableMemoryAllocator::GetInstance()
->AllocateLockedDiscardableMemory(bytes));
Which would seem to indicate that GetInstance() returned null? But if it were null, that should happen any time we run this test, unless the image itself is causing a problem. It's possible we're stomping memory when we try to initialize the pixels, though I don't see anything obvious in the code that we're doing wrong.
I've uploaded https://skia-review.googlesource.com/c/skia/+/171006. This makes it so that we don't try to fill the pixels on this particular image. If applying this patch fixes the problem, this would suggest that we are stomping memory. (It wouldn't fix the problem in general, though it might be something we should submit anyway.)
,
Nov 14
The patch doesn't seem to prevent the issue when I applied it locally.
,
Nov 14
Thanks for trying! Since it didn't make a difference, I don't see how the decode itself could be relevant.
,
Nov 14
>But if it were null, that should happen any time we run this test I wouldn't be so shocked if that happened. The crash seems to repro no matter what the 4th byte is. I'm more suspicious that this is a problem with the way I've built this fuzzer because of the fact that this bug occurs in chromium source code and not skia source.
,
Nov 14
> The crash seems to repro no matter what the 4th byte is. Oh, I don't think I was clear - I meant that if GetInstance() was returning null, we should get a similar crash no matter what the image is - a valid image would cause the same problem, since we'd still have to call GetInstance().
,
Nov 14
>Oh, I don't think I was clear - I meant that if GetInstance() was returning null, we should get a similar crash no matter what the image is - a valid image would cause the same problem, since we'd still have to call GetInstance(). This does seem to be the case. I was able to prevent this problem by setting a discardable memory allocator before fuzzing starts. I'm going to do a bit more investigating and then put up a patch.
,
Nov 14
Ok, great, thanks.
,
Nov 14
Issue 905275 has been merged into this issue.
,
Nov 14
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d0984eb5712e3c0e0eeba5d99d73b8414c02d4e commit 8d0984eb5712e3c0e0eeba5d99d73b8414c02d4e Author: Jonathan Metzman <metzman@chromium.org> Date: Mon Nov 19 17:15:06 2018 [skia] Initialize allocator properly before fuzzing and move targets Set the discardable memory allocator before fuzzing to avoid spurious null derefs when discardable memory is allocated. Also, move fuzzer build config to //skia/tools/fuzzers/BUILD.gn Bug: 903632, 904613 Change-Id: Ib987333706309eb470bcb988c53ddeebf5f5851e Reviewed-on: https://chromium-review.googlesource.com/c/1336430 Commit-Queue: Jonathan Metzman <metzman@chromium.org> Reviewed-by: Leon Scroggins <scroggo@chromium.org> Cr-Commit-Position: refs/heads/master@{#609339} [modify] https://crrev.com/8d0984eb5712e3c0e0eeba5d99d73b8414c02d4e/skia/BUILD.gn [add] https://crrev.com/8d0984eb5712e3c0e0eeba5d99d73b8414c02d4e/skia/tools/fuzzers/BUILD.gn [add] https://crrev.com/8d0984eb5712e3c0e0eeba5d99d73b8414c02d4e/skia/tools/fuzzers/fuzzer_environment.cc
,
Nov 19
ClusterFuzz has detected this issue as fixed in range 609334:609340. Detailed report: https://clusterfuzz.com/testcase?key=5515976856305664 Fuzzer: libFuzzer_skia_image_decode_fuzzer Job Type: windows_libfuzzer_chrome_asan Platform Id: windows Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: SkDiscardableMemory::Create SkBitmapCache::Alloc SkImage_Lazy::getROPixels Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_libfuzzer_chrome_asan&range=607281:607292 Fixed: https://clusterfuzz.com/revisions?job=windows_libfuzzer_chrome_asan&range=609334:609340 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5515976856305664 See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing_on_windows.md for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Nov 19
ClusterFuzz testcase 5515976856305664 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Dec 14
The documentation for reproducing bugs on Windows was moved to: https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by kkaluri@chromium.org
, Nov 13