Bug 285388

Summary: Reduce use of WTF_ALLOW_UNSAFE_BUFFER_USAGE in WebCore/platform/image-decoders
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 285458    
Bug Blocks:    

Chris Dumez
Reported 2025-01-04 15:10:18 PST
Reduce use of WTF_ALLOW_UNSAFE_BUFFER_USAGE in WebCore/platform/image-decoders.
Attachments
Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2025-01-04 15:14:01 PST
Pull request: https://github.com/WebKit/WebKit/pull/38536
EWS
Comment 2 2025-01-05 14:58:13 PST
Committed 288451@main (334ca2793400): <https://commits.webkit.org/288451@main> Reviewed commits have been landed. Closing PR #38536 and removing active labels.
Radar WebKit Bug Importer
Comment 3 2025-01-05 14:59:16 PST
<rdar://problem/142388765>
Michael Catanzaro
Comment 4 2025-01-06 12:31:39 PST
This didn't work unfortunately. I suggest we just revert for now. This can't be tested on EWS since the EWS mostly use GCC. Here are the regressions I see with Clang 19. GIFImageDecoder: /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:170:78: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 170 | const Vector<ScalableImageDecoderFrame>::iterator end(m_frameBufferCache.begin() + clearBeforeFrame); | ~~~~~~~~~~~~~~~~~~~^~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:190:49: error: 'i' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] 190 | Vector<ScalableImageDecoderFrame>::iterator i(end); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:191:158: note: used in pointer arithmetic here 191 | for (; (i != m_frameBufferCache.begin()) && (i->isInvalid() || (i->disposalMethod() == ScalableImageDecoderFrame::DisposalMethod::RestoreToPrevious)); --i) { | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:197:54: error: 'j' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] 197 | for (Vector<ScalableImageDecoderFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:197:95: note: used in pointer arithmetic here 197 | for (Vector<ScalableImageDecoderFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) { | ^ GIFImageReader: /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:452:29: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 452 | m_screenWidth = GETINT16(currentComponent.data()); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:453:30: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 453 | m_screenHeight = GETINT16(currentComponent.data() + 2); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:453:56: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 453 | m_screenHeight = GETINT16(currentComponent.data() + 2); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:453:56: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 453 | m_screenHeight = GETINT16(currentComponent.data() + 2); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:35: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:587:39: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 587 | currentFrame->delayTime = GETINT16(currentComponent.data() + 1) * 10; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:587:65: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 587 | currentFrame->delayTime = GETINT16(currentComponent.data() + 1) * 10; | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:587:65: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 587 | currentFrame->delayTime = GETINT16(currentComponent.data() + 1) * 10; | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:35: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:631:31: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 631 | m_loopCount = GETINT16(currentComponent.data() + 1); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:631:57: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 631 | m_loopCount = GETINT16(currentComponent.data() + 1); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:631:57: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 631 | m_loopCount = GETINT16(currentComponent.data() + 1); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:35: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:656:23: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 656 | xOffset = GETINT16(currentComponent.data()); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:657:23: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 657 | yOffset = GETINT16(currentComponent.data() + 2); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:657:49: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 657 | yOffset = GETINT16(currentComponent.data() + 2); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:657:49: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 657 | yOffset = GETINT16(currentComponent.data() + 2); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:35: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:660:22: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 660 | width = GETINT16(currentComponent.data() + 4); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:660:48: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 660 | width = GETINT16(currentComponent.data() + 4); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:660:48: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 660 | width = GETINT16(currentComponent.data() + 4); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:35: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:661:22: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 661 | height = GETINT16(currentComponent.data() + 6); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:24: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:661:48: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 661 | height = GETINT16(currentComponent.data() + 6); | ~~~~~~~~~~~~~~~~~^~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp:103:25: note: expanded from macro 'GETINT16' 103 | #define GETINT16(p) ((p)[1]<<8|(p)[0]) | ^ fatal error: too many errors emitted, stopping now [-ferror-limit=] JPEGImageDecoder: /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:213:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 213 | && marker->data[1] == 'C' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:214:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 214 | && marker->data[2] == 'C' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:215:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 215 | && marker->data[3] == '_' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:216:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 216 | && marker->data[4] == 'P' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:217:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 217 | && marker->data[5] == 'R' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:218:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 218 | && marker->data[6] == 'O' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:219:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 219 | && marker->data[7] == 'F' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:220:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 220 | && marker->data[8] == 'I' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:221:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 221 | && marker->data[9] == 'L' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:222:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 222 | && marker->data[10] == 'E' | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:223:12: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 223 | && marker->data[11] == '\0'; | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:233:35: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 233 | unsigned sequenceNumber = marker->data[12]; | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:237:32: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage] 237 | unsigned markerCount = marker->data[13]; | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:242:23: error: the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information [-Werror,-Wunsafe-buffer-usage-in-container] 242 | buffer.append(std::span { reinterpret_cast<const uint8_t*>(marker->data + iccHeaderSize), markerSize }); | ^ /home/mcatanzaro/Projects/WebKit/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:242:68: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] 242 | buffer.append(std::span { reinterpret_cast<const uint8_t*>(marker->data + iccHeaderSize), markerSize }); | ^~~~~~~~~~~~ 15 errors generated.
Michael Catanzaro
Comment 5 2025-01-06 12:32:50 PST
Reopened Bugzilla. Introduced many -Werror=unsafe-buffer-usage failures, tracking revert in https://bugs.webkit.org/show_bug.cgi?id=285458.
Michael Catanzaro
Comment 6 2025-01-06 13:37:53 PST
I will follow up on this via bug #285462.
Note You need to log in before you can comment on or make changes to this bug.