Security: Skia: Uninitialized variable in SkPipeDeserializer::readImage
Reported by
quangn...@gmail.com,
Sep 19
|
||||||||
Issue descriptionVULNERABILITY DETAILS I have found a vulnerability in SkPipeDeserializer::readImage. The attached poc can crash skia. the following line, SkPipeDeserializer::readImage will check `packedVerb` to initialize `fImpl->fImages`. if the `verb` is not matched, `fImpl->fImages` is empty set and `nullptr` value should be returned: https://cs.chromium.org/chromium/src/third_party/skia/src/pipe/SkPipeReader.cpp?g=0&l=810 ``` if (SkPipeVerb::kDefinePicture == unpack_verb(packedVerb)) { SkPipeInflator inflator(&fImpl->fImages, &fImpl->fPictures, &fImpl->fTypefaces, &fImpl->fFactories, fImpl->fProcs); SkPipeReader reader(this, ptr, size); reader.setInflator(&inflator); definePicture_handler(reader, packedVerb, nullptr); packedVerb = reader.read32(); // read the next verb } else { // Because fImpl->fImages is empty set, you should return nullptr return nullptr; } ``` VERSION Operating System: Linux REPRODUCTION CASE Build Skia with asserts disabled ``` bin/gn gen out/Release --args='is_debug=false' ninja -C out/Release ``` Run testcase: ``` ./out/Release/fuzz -t pipe -b SkPicture_set_crash ``` Crate state: ``` Stopped reason: SIGSEGV 0x0000000000c2541e in sk_sp<SkPicture>::get (this=0x88) at ../../include/core/SkRefCnt.h:296 296 T* get() const { return fPtr; } gdb-peda$ bt #0 0x0000000000c2541e in sk_sp<SkPicture>::get (this=0x88) at ../../include/core/SkRefCnt.h:296 #1 0x0000000000e5cde4 in SkRefSet<SkPicture>::get (this=0x25eefd8, index=0x11) at ../../src/pipe/SkRefSet.h:18 #2 0x0000000000e5cab6 in SkPipeDeserializer::readPicture (this=0x7fffffffcd20, data=0x7ffff7ff6000, size=0x1c5) at ../../src/pipe/SkPipeReader.cpp:827 #3 0x0000000000c34a5d in SkPipeDeserializer::readPicture (this=0x7fffffffcd20, data=0x25eea10) at ../../src/core/SkPipe.h:63 #4 0x0000000000c38741 in fuzz_skpipe (bytes=...) at ../../fuzz/FuzzMain.cpp:614 #5 0x0000000000c35c7a in fuzz_file (path=..., type=...) at ../../fuzz/FuzzMain.cpp:189 #6 0x0000000000c35578 in main (argc=0x5, argv=0x7fffffffe158) at ../../fuzz/FuzzMain.cpp:109 #7 0x00007ffff6061830 in __libc_start_main (main=0xc35447 <main(int, char**)>, argc=0x5, argv=0x7fffffffe158, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe148) at ../csu/libc-start.c:291 #8 0x0000000000c1a109 in _start () ``` PATCH patch for this issue: ``` diff --git a/src/pipe/SkPipeReader.cpp b/src/pipe/SkPipeReader.cpp index 78db581..0cdfb2d 100644 --- a/src/pipe/SkPipeReader.cpp +++ b/src/pipe/SkPipeReader.cpp @@ -816,6 +816,11 @@ sk_sp<SkPicture> SkPipeDeserializer::readPicture(const void* data, size_t size) definePicture_handler(reader, packedVerb, nullptr); packedVerb = reader.read32(); // read the next verb } + else { + SkDebugf("-------- unexpected verb for readPicture %d\n", unpack_verb(packedVerb)); + return nullptr; + } + if (SkPipeVerb::kWritePicture != unpack_verb(packedVerb)) { SkDebugf("-------- unexpected verb for readPicture %d\n", unpack_verb(packedVerb)); return nullptr; ```
,
Sep 21
,
Sep 21
I don't think anything actually uses SkPipe. +mtklein to confirm
,
Sep 21
Kevin, should the fuzz target for SkPipe be removed or maybe a runtime warning displayed that don't file bugs from that.
,
Sep 21
Right, SkPipe is not used. I believe we started fuzzing it only as a way to get a head start on anything that might want to start using it. I have at least half a mind to delete it all.
,
Sep 21
,
Sep 21
Bulk edit of recently filed skia pipe bugs.
,
Sep 21
The following revision refers to this bug: https://skia.googlesource.com/skia/+/60900b55f9e1d7e566449c923f2a8104f53f2d04 commit 60900b55f9e1d7e566449c923f2a8104f53f2d04 Author: Mike Klein <mtklein@google.com> Date: Fri Sep 21 17:20:25 2018 move skpipe to experimental Nothing's using it except test tools. I'd like to make that a bit clearer by getting it out of src. Disabled the fuzzer. Removed the bench so Android's building nanobench doesn't block this. Bug: chromium:886713 Change-Id: I761f52c40171c27ff4b699409b32647e84684ec3 Reviewed-on: https://skia-review.googlesource.com/156240 Commit-Queue: Mike Klein <mtklein@google.com> Reviewed-by: Kevin Lubick <kjlubick@google.com> [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkPipeReader.cpp [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/fuzz/FuzzMain.cpp [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/bench/RecordingBench.cpp [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkPipeCanvas.h [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/bench/RecordingBench.h [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/BUILD.gn [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/public.bzl [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/gn/core.gni [modify] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/bench/nanobench.cpp [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkPipeCanvas.cpp [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkPipeFormat.h [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkPipe.h [rename] https://crrev.com/60900b55f9e1d7e566449c923f2a8104f53f2d04/experimental/pipe/SkRefSet.h
,
Sep 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44728781557ee3152554ab5ba2e09e4a30ad9171 commit 44728781557ee3152554ab5ba2e09e4a30ad9171 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Sat Sep 22 01:26:19 2018 Roll src/third_party/skia f610bae66163..11f4994b84e1 (32 commits) https://skia.googlesource.com/skia.git/+log/f610bae66163..11f4994b84e1 git log f610bae66163..11f4994b84e1 --date=short --no-merges --format='%ad %ae %s' 2018-09-21 brianosman@google.com Revert "Reland "Preserve colorType and alphaType in SkImage::makeColorSpace"" 2018-09-21 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader 9f6c0b899ae6..c5e68abdea9e (1 commits) 2018-09-21 brianosman@google.com Cleanup SkPatchUtils, stop using SkColorSpaceXform 2018-09-21 mtklein@google.com focus exported color apis 2018-09-21 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/angle2 ab5fb5edb186..9d84ccbaf233 (1 commits) 2018-09-21 caryclark@skia.org move color4f docs to in progress 2018-09-21 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader 4daedcd8c612..9f6c0b899ae6 (2 commits) 2018-09-21 brianosman@google.com Introduce SkRGBA4f, templated on SkAlphaType 2018-09-21 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader c89f75b65555..4daedcd8c612 (1 commits) 2018-09-21 halcanary@google.com Docs: update to docs from a4daf19319 2018-09-21 halcanary@google.com SK_SUPPORT_LEGACY_DOCUMENT_FACTORY: clean up part 1/2 2018-09-21 herb@google.com Regularize path cache lookup with Mask and Fallback 2018-09-21 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/swiftshader 2ecc8b712b08..c89f75b65555 (1 commits) 2018-09-21 jvanverth@google.com Optimize SkTriangulateSimplePolygon. 2018-09-21 robertphillips@google.com Fix Chrome iOS builder 2018-09-21 caryclark@skia.org sync color4f with docs 2018-09-21 lsalzman@mozilla.com mark SkCanvas layer device as immutable before compositing it on restore 2018-09-21 brianosman@google.com Reland "Preserve colorType and alphaType in SkImage::makeColorSpace" 2018-09-21 caryclark@skia.org doc change for getAlphaf 2018-09-21 reed@google.com Reland "add getAlphaf() to pixmap/bitmap" 2018-09-21 mtklein@google.com move skpipe to experimental 2018-09-21 kjlubick@google.com [canvaskit] Link demos to fiddles 2018-09-21 kjlubick@google.com Update lottie samples 2018-09-21 mtklein@google.com Reland "have SkConvertPixels use SkColorSpaceXformSteps" 2018-09-21 brianosman@google.com Revert "Preserve colorType and alphaType in SkImage::makeColorSpace" 2018-09-21 brianosman@google.com Work toward removing SkPM4f 2018-09-21 caryclark@skia.org shorten animation to work around fiddlecli bug 2018-09-21 bungeman@google.com Use SkUniqueCFRef in SkImage*CG. 2018-09-21 jvanverth@google.com Fix line distance issue 2018-09-21 brianosman@google.com Preserve colorType and alphaType in SkImage::makeColorSpace 2018-09-21 brianosman@google.com Set window dimensions with GetClientRect, not GetWindowRect 2018-09-21 robertphillips@google.com Add SkImage_Base API to access planar data Created with: gclient setdep -r src/third_party/skia@11f4994b84e1 The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:886713 TBR=reed@chromium.org Change-Id: I53fd65f7522d77b7cafeaf87380d179511e2502e Reviewed-on: https://chromium-review.googlesource.com/1239654 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#593408} [modify] https://crrev.com/44728781557ee3152554ab5ba2e09e4a30ad9171/DEPS
,
Sep 22
,
Dec 29
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 |
||||||||
Comment 1 by quangn...@gmail.com
, Sep 21