RESOLVED INVALID219630
[GTK] Add an overflow check to remove the gcc warning since r270506. 
https://bugs.webkit.org/show_bug.cgi?id=219630
Summary [GTK] Add an overflow check to remove the gcc warning since r270506.
Joonghun Park
Reported 2020-12-08 00:43:43 PST
It seems that it needs to add an overflow check and ASSERT to remove the gcc warning below. warning: ‘void* memcpy(void*, const void*, size_t)’ specified bound 18446744073709551614 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
Attachments
Patch (1.50 KB, patch)
2020-12-08 00:46 PST, Joonghun Park 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joonghun Park
Comment 1 2020-12-08 00:46:25 PST
Created attachment 415616 [details] Patch
Darin Adler
Comment 2 2020-12-08 12:58:53 PST
Comment on attachment 415616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415616&action=review > Source/WebCore/Modules/mediastream/H264Utils.cpp:138 > + if (spsPpsLength > std::numeric_limits<size_t>::max() - 2) { > + ASSERT_NOT_REACHED(); > + return { }; > + } Vector::resize will call CRASH if the size is too large. This instead returns an empty buffer. I don’t think there’s a good reason to be inconsistent about this. So this should just be CRASH(). An alternate way to do this would be to use checked arithmetic. CheckedSize exists for cases like this. That fix would be below. > Source/WebCore/Modules/mediastream/H264Utils.cpp:141 > buffer.resize(spsPpsLength + 2); Could do this instead: buffer.grow((CheckedSize(spsPpsLength) + 2).unsafeGet()); Using unsafeGet gives us the same crash behavior as Vector::resize. Using Vector::grow instead of Vector::resize is the best practice when starting with a new vector, skips unneeded logic for shrinking the buffer. Doing it this way has the advantage of tying the check directly to the arithmetic instead of having to make sure the preflight matches the arithmetic below.
Joonghun Park
Comment 3 2021-09-11 00:25:04 PDT
This file and the statement caused the build warning no longer exist, so the build warning was removed.
Note You need to log in before you can comment on or make changes to this bug.