Bug 274027

Summary: REGRESSION(277476@main): [GTK] Crash in WebCore::GIFImageDecoder::haveDecodedRow
Product: WebKit Reporter: Kdwk <kdwkleung>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cdumez, cgarcia, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=272640
Attachments:
Description Flags
gdb (bt full; c).txt none

Kdwk
Reported 2024-05-10 18:58:48 PDT
Created attachment 471366 [details] gdb (bt full; c).txt WebKitGTK 2.45.1 crashes on many websites, including bugs.webkit.org, in WebCore::GIFImageDecoder::haveDecodedRow.
Attachments
gdb (bt full; c).txt (87.64 KB, text/plain)
2024-05-10 18:58 PDT, Kdwk
no flags
Carlos Garcia Campos
Comment 1 2024-05-13 03:02:53 PDT
Why is this related to skia? This could have regressed in https://commits.webkit.org/277476@main
Michael Catanzaro
Comment 2 2024-05-13 05:38:32 PDT
This crash is happening 100% of the time when trying to load any page on this Bugzilla. Also on a very large number of other websites. I had to downgrade to post this comment.
Michael Catanzaro
Comment 3 2024-05-13 07:07:22 PDT
This is a libstdc++ assertion failure due to buffer overflow when indexing a std::span, so 277476@main is a very likely suspect. The assertion is: /usr/bin/../lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/span:286: reference std::span<const unsigned char>::operator[](size_type) const [_Type = const unsigned char, _Extent = 18446744073709551615]: Assertion '__idx < size()' failed To reproduce, build with -DCMAKE_CXX_FLAGS="-Wp,-D_GLIBCXX_ASSERTIONS"
Michael Catanzaro
Comment 4 2024-05-13 07:24:37 PDT
Confirmed, this regressed in 277476@main
Michael Catanzaro
Comment 5 2024-05-13 08:16:58 PDT
There is a preexisting buffer overread here in GIFImageDecoder::haveDecodedRow: const size_t colorIndex = static_cast<size_t>(sourceValue) * 3; buffer.backingStore()->setPixel(currentAddress, colorMap[colorIndex], colorMap[colorIndex + 1], colorMap[colorIndex + 2], 255); Here the values of colorIndex are in practice much larger than the values of colorMapSize.
Michael Catanzaro
Comment 6 2024-05-13 09:11:03 PDT
Actually I think I'm wrong about there being a preexisting bug. The semantic "size" of the color map was actually three times its size in bytes. This explains why the code was willing to read exactly 1 or 2 bytes past the "end" of the color map.
Michael Catanzaro
Comment 7 2024-05-13 09:21:55 PDT
(In reply to Michael Catanzaro from comment #6) > The semantic "size" of the color map was actually three times its size in bytes. Sorry, I mean its semantic size was one third its size in bytes. After 277476@main, only the first third of the color map is still available and the rest of the map is missing.
Michael Catanzaro
Comment 8 2024-05-13 09:29:16 PDT
EWS
Comment 9 2024-05-14 00:34:46 PDT
Committed 278739@main (cae3dbd2f345): <https://commits.webkit.org/278739@main> Reviewed commits have been landed. Closing PR #28477 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.