New issue
Advanced search Search tips

Issue 887912 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Sep 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: ----
Type: Bug-Security



Sign in to add a comment

Security: Skia: Invalid parameter handling in SkPipeReader::findFactory

Reported by quangn...@gmail.com, Sep 21

Issue description

VULNERABILITY DETAILS

There is an undefined behavior (strcmp with null pointer) in SkPipeReader::findFactory function (src/pipe/SkPipeReader.cpp), which can cause denial of service (crash) or possibly other unspecified impacts via a crafted input. Developer should check `name` value before using it.

https://cs.chromium.org/chromium/src/third_party/skia/src/pipe/SkPipeReader.cpp?g=0&l=202

```
   SkFlattenable::Factory findFactory(const char name[]) {
        SkFlattenable::Factory factory;
        // Check if a custom Factory has been specified for this flattenable.
        if (!(factory = this->getCustomFactory(SkString(name)))) { // SHOULD CHECK BEFORE USING IT
            // If there is no custom Factory, check for a default.
            factory = SkFlattenable::NameToFactory(name);
        }
        return factory;
    }
```


VERSION
Operating System: Linux
Skia version: I use latest version in github(https://github.com/google/skia), commit https://github.com/google/skia/commit/05cf051f0252c601eefb32cfb58738d15bdc0c5d


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 SkPipeReader_findFactory_null_name
```

Crate state:
```
Program received signal SIGSEGV, Segmentation fault.
__strcmp_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:32
32      ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S: No such file or directory.
(gdb) bt
#0  __strcmp_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S:32
#1  0x0000000000d5706a in (anonymous namespace)::EntryComparator::operator() (this=0x7fffffffc8b0, a=..., b=0x0) at ../../src/core/SkFlattenable.cpp:66
#2  0x0000000000d57d5a in __gnu_cxx::__ops::_Iter_comp_val<(anonymous namespace)::EntryComparator>::operator()<(anonymous namespace)::Entry*, char const* const> (this=0x7fffffffc8b0, __it=0x25d53a8 <(anonymous namespace)::gEntries+840>, __val=@0x7fffffffc928: 0x0) at /usr/include/c++/5/bits/predefined_ops.h:144
#3  0x0000000000d579e1 in std::__equal_range<(anonymous namespace)::Entry*, char const*, __gnu_cxx::__ops::_Iter_comp_val<(anonymous namespace)::EntryComparator>, __gnu_cxx::__ops::_Val_comp_iter<(anonymous namespace)::EntryComparator> > (__first=0x25d5060 <(anonymous namespace)::gEntries>, __last=0x25d56f0 <(anonymous namespace)::gEntries+1680>, __val=@0x7fffffffc928: 0x0, __comp_it_val=..., __comp_val_it=...) at /usr/include/c++/5/bits/stl_algo.h:2141
#4  0x0000000000d577e0 in std::equal_range<(anonymous namespace)::Entry*, char const*, (anonymous namespace)::EntryComparator> (__first=0x25d5060 <(anonymous namespace)::gEntries>, __last=0x25d56f0 <(anonymous namespace)::gEntries+1680>, __val=@0x7fffffffc928: 0x0, __comp=...) at /usr/include/c++/5/bits/stl_algo.h:2237
#5  0x0000000000d57202 in SkFlattenable::NameToFactory (name=0x0) at ../../src/core/SkFlattenable.cpp:114
#6  0x0000000000e5b982 in SkPipeReader::findFactory (this=0x7fffffffcbd0, name=0x0) at ../../src/pipe/SkPipeReader.cpp:204
#7  0x0000000000e5deb8 in defineFactory_handler (reader=..., packedVerb=570754304, canvas=0x25f2640) at ../../src/pipe/SkPipeReader.cpp:665
#8  0x0000000000e5e8c6 in do_playback (reader=..., canvas=0x25f2640, endPictureIndex=0x7fffffffcaa8) at ../../src/pipe/SkPipeReader.cpp:855
#9  0x0000000000e5dfe6 in definePicture_handler (reader=..., packedVerb=587202560, canvas=0x0) at ../../src/pipe/SkPipeReader.cpp:686
#10 0x0000000000e5e6b1 in SkPipeDeserializer::readPicture (this=0x7fffffffcd20, data=0x7ffff7ff6000, size=112) at ../../src/pipe/SkPipeReader.cpp:816
#11 0x0000000000c34f89 in SkPipeDeserializer::readPicture (this=0x7fffffffcd20, data=0x25f1a10) at ../../src/core/SkPipe.h:63
#12 0x0000000000c38c6d in fuzz_skpipe (bytes=...) at ../../fuzz/FuzzMain.cpp:614
#13 0x0000000000c361a6 in fuzz_file (path=..., type=...) at ../../fuzz/FuzzMain.cpp:189
#14 0x0000000000c35aa4 in main (argc=5, argv=0x7fffffffe158) at ../../fuzz/FuzzMain.cpp:109
```

PATCH

patch for this issue:
```
--- a/src/pipe/SkPipeReader.cpp
+++ b/src/pipe/SkPipeReader.cpp
@@ -198,6 +212,8 @@ public:

     SkFlattenable::Factory findFactory(const char name[]) {
         SkFlattenable::Factory factory;
+        if (!name)
+            return nullptr;
         // Check if a custom Factory has been specified for this flattenable.
         if (!(factory = this->getCustomFactory(SkString(name)))) {
             // If there is no custom Factory, check for a default.
```
 
Cc: kjlubick@chromium.org hcm@google.com
Components: Internals>Skia
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Not sure if this code is reachable from Chrome (guessing this because we don't have fuzzer in OSS-Fuzz) but Skia people can confirm.
Cc: mtklein@chromium.org reed@google.com
Labels: Security_Impact-None
Status: Available (was: Unconfirmed)
Bulk edit of recently filed skia pipe bugs.
Status: WontFix (was: Available)
See https://bugs.chromium.org/p/chromium/issues/detail?id=886713#c8
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 29

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