Bug 190281

Summary: Validation in Connection::readBytesFromSocket() is too aggressive
Product: WebKit Reporter: Konstantin Tokarev <annulen>
Component: PlatformAssignee: Konstantin Tokarev <annulen>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, mcatanzaro, webkit-bug-importer, zan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=171871
Attachments:
Description Flags
Patch none

Konstantin Tokarev
Reported 2018-10-04 10:06:03 PDT
Since r217206 Connection::readBytesFromSocket() validates size of control message. However, it compares cmsg_len with attachmentMaxAmount, while Connection::sendOutgoingMessage() computes it as CMSG_LEN(sizeof(int) * attachmentFDBufferLength) where attachmentFDBufferLength <= attachmentMaxAmount. This mismatch between sender and receiver leads to possibility of assertion failure with large number of attachments, e.g. here 62 attachments have cmsg_length == 264.
Attachments
Patch (2.20 KB, patch)
2018-10-04 10:09 PDT, Konstantin Tokarev 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Konstantin Tokarev
Comment 1 2018-10-04 10:09:04 PDT
Created attachment 351598 [details] Patch
Michael Catanzaro
Comment 2 2018-10-07 12:54:55 PDT
Comment on attachment 351598 [details] Patch Oh wow, good find. Under what scenario were you hitting this failure? Any way to write a test? Can you add it to https://trac.webkit.org/wiki/WebKitGTK/2.22.x (for 2.22.3) after landing, please?
Konstantin Tokarev
Comment 3 2018-10-08 11:45:37 PDT
(In reply to Michael Catanzaro from comment #2) > Comment on attachment 351598 [details] > Patch > > Oh wow, good find. > > Under what scenario were you hitting this failure? It was reproducing with QtWebKit, but not with GTK port. I guess behavior is different because Qt uses UI-side compositing, which is probably a reson why there are so many attachments. >Any way to write a test? No idea from the top of my had. > > Can you add it to https://trac.webkit.org/wiki/WebKitGTK/2.22.x (for 2.22.3) > after landing, please? Sure
WebKit Commit Bot
Comment 4 2018-10-08 12:12:36 PDT
Comment on attachment 351598 [details] Patch Clearing flags on attachment: 351598 Committed r236928: <https://trac.webkit.org/changeset/236928>
WebKit Commit Bot
Comment 5 2018-10-08 12:12:37 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 6 2018-10-08 12:13:28 PDT
<rdar://problem/45098148>
Note You need to log in before you can comment on or make changes to this bug.