|Summary:||Crash in legacy WebView due to unguarded makeSimpleRange dereference|
|Product:||WebKit||Reporter:||Marc Haisenko <haisenko>|
|Component:||HTML Editing||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||darin, megan_gardner, rniwa, webkit-bug-importer, wenson_hsieh|
Description Marc Haisenko 2021-06-24 00:00:08 PDT
Created attachment 432136 [details] Crash log 1 Starting with macOS 11.3, we are seeing a lot more WebKit crashes in our app that uses the legacy WebView. With the help of the crash reports (which already told us in which method the crash is happening), disassembler and WebKit sources I was able to trace the crash to two lines in `WebEditorClient::requestCandidatesForSelection`. They call `makeSimpleRange` and dereference the result, assuming the return value is never NULL. Unfortunately, it now sometimes is and thus crashes (_why_ that is happening now may also be interesting, apart from fixing the crash). See lines 1110 and 1111 in WebEditorClient.mm. https://trac.webkit.org/browser/webkit/trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.mm?rev=278253#L1110 auto selectionStartOffsetInParagraph = characterCount(*makeSimpleRange(startOfParagraph(selectionStart), selectionStart)); auto selectionLength = characterCount(*makeSimpleRange(selectionStart, selection.visibleEnd())); We see this crash on macOS 11.3 and newer, both Intel and M1. (It might have happened on earlier versions as well but there definitely is an increase since macOS 11.3.) Unfortunately, I don't have a recipe for reproducing the crash yet (we are not able to reproduce it in-house but see lots of crash reports from users and have users that contact us because of this). It seems there are multiple paths leading to this crash, but they all end up crashing in `WebEditorClient::requestCandidatesForSelection + 2505` on Intel (macOS 11.4) or `WebEditorClient::requestCandidatesForSelection + 2528` on M1 (macOS 11.4).
Comment 1 Darin Adler 2021-06-24 14:04:08 PDT
Easy to fix this by adding the null check even if we can’t reproduce.
Comment 2 Darin Adler 2021-06-24 14:04:25 PDT
Would be nice to know why it’s happening.
Comment 3 Darin Adler 2021-06-24 14:05:47 PDT
Putting this in the editing component since it’s related to selection. Someone should do the "add null checks" patch. I think we can just early return if either simple range is null.