Bug 176303

Summary: [GTK][WPE] UI process crash in WebBackForwardList::restoreFromState
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, berto, bugs-noreply, buildbot, cgarcia, darin, gustavo, kling, mcatanzaro, sam, thorton
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1458818
https://bugzilla.gnome.org/show_bug.cgi?id=785557
Attachments:
Description Flags
Backtrace from Red Hat Bugzilla
none
Backtrace from GNOME Bugzilla
none
Patch
achristensen: review-
Patch mcatanzaro: review+

Description Michael Catanzaro 2017-09-03 07:24:02 PDT 
Created attachment 319775 [details]
Backtrace from Red Hat Bugzilla

I have 45 reports of this crash in Fedora, but only from 10 unique users. This crash is particularly nasty because once you get into this state, Epiphany will crash immediately on startup until you delete your session_state.xml:

Truncated backtrace:
Thread no. 1 (10 frames)
 #0 WTFCrash at /usr/src/debug/webkitgtk-2.16.3/Source/WTF/wtf/Assertions.cpp:323
 #1 WTF::CrashOnOverflow::crash at /usr/src/debug/webkitgtk-2.16.3/Source/WTF/wtf/CheckedArithmetic.h:85
 #2 WTF::CrashOnOverflow::overflowed at /usr/src/debug/webkitgtk-2.16.3/Source/WTF/wtf/CheckedArithmetic.h:78
 #3 WTF::Vector<WTF::RefPtr<WebKit::WebBackForwardListItem>, 0ul, WTF::CrashOnOverflow, 16ul>::at at /usr/src/debug/webkitgtk-2.16.3/Source/WTF/wtf/Vector.h:661
 #4 WTF::Vector<WTF::RefPtr<WebKit::WebBackForwardListItem>, 0ul, WTF::CrashOnOverflow, 16ul>::operator[] at /usr/src/debug/webkitgtk-2.16.3/Source/WTF/wtf/Vector.h:676
 #5 WebKit::WebBackForwardList::backItem at /usr/src/debug/webkitgtk-2.16.3/Source/WebKit2/UIProcess/WebBackForwardList.cpp:222
 #6 WebKit::WebPageProxy::restoreFromSessionState at /usr/src/debug/webkitgtk-2.16.3/Source/WebKit2/UIProcess/WebPageProxy.cpp:2442
 #7 webkit_web_view_restore_session_state at /usr/src/debug/webkitgtk-2.16.3/Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:3791
 #8 load_delayed_request_if_mapped at ephy-embed.c:650
 #9 g_timeout_dispatch

Two backtraces attached, from separate users.

I worked a bit with this with Kai from the GNOME bug report at GUADEC, saving the bad Ephy session state and messing with it to figure out what went wrong. See the downstream GNOME bug for more details. Here is the problematic embed state. The first version is good and the second version, which got saved into Kai's session state, is bad.
14c14
< 	 	 <embed url="https://dstip.zuv.tu-berlin.de/student/applications/3539?step=6" title="Probleme beim Laden der Seite" loading="true" history="AQAAAAAAAACrAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L3NpZ25faW4AaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvdmVyaWZ5X3JlZ2lzdHJhdGlvbi9lNzdkM2UxMjUxNGYyMjk5ZDIzYWZhMjg0NjkxMGNmYTNjOGZhYmI5P3N0ZGlkPTMzNTQAAACP88YP1VQFAI7zxg/VVAUAAAAAAAAAAAAAAAAAAADwP8CgoKCfni8AAQAAAP8ANACtAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5L2VkaXQAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvc2lnbl9pbgBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9zaWduX2luAAAAAACR88YP1VQFAJDzxg/VVAUAAAAAAJoBAAAAAAAAAADwP8CdnZ2cbT4AAAAAAP8ANACvAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5L2VkaXQ/c3RlcD0yAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5P3N0ZXA9MQBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0AAAAAAAAk/PGD9VUBQCS88YP1VQFAAAAAADQAwAAAAAAAAAA8D/oxMTEw4VFAAAAAAAnATQAsQMAAAAAAABEZXV0c2NobGFuZHN0aXBlbmRpdW0g4oCiIEJld2VyYnVuZyBhYmdlYmVuAAAAAABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0P3N0ZXA9MwBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOT9zdGVwPTIAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvYXBwbGljYXRpb25zLzM1MzkvZWRpdD9zdGVwPTIAAAAAAAAAlfPGD9VUBQCU88YP1VQFAAAAAADQAAAAAAAAAAAA8D/wy8vLyoVFAAAAAAAvATQAswMAAAAAAABEZXV0c2NobGFuZHN0aXBlbmRpdW0g4oCiIEJld2VyYnVuZyBhYmdlYmVuAAAAAABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0P3N0ZXA9NABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOT9zdGVwPTMAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvYXBwbGljYXRpb25zLzM1MzkvZWRpdD9zdGVwPTMAAAAAAAAAl/PGD9VUBQCW88YP1VQFAAAAAACYAAAAAAAAAAAA8D/wy8vLyoVFAAAAAAAvATQACAEQAkADeASwBQAASwAAAMIF"/>
---
> 	 	 <embed url="https://dstip.zuv.tu-berlin.de/student/applications/3539?step=6" title="Probleme beim Laden der Seite" history="AQAAAAAAAACrAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L3NpZ25faW4AaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvdmVyaWZ5X3JlZ2lzdHJhdGlvbi9lNzdkM2UxMjUxNGYyMjk5ZDIzYWZhMjg0NjkxMGNmYTNjOGZhYmI5P3N0ZGlkPTMzNTQAAACP88YP1VQFAI7zxg/VVAUAAAAAAAAAAAAAAAAAAADwP8CgoKCfni8AAQAAAP8ANACtAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5L2VkaXQAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvc2lnbl9pbgBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9zaWduX2luAAAAAACR88YP1VQFAJDzxg/VVAUAAAAAAJoBAAAAAAAAAADwP8CdnZ2cbT4AAAAAAP8ANACvAwAAAAAAAERldXRzY2hsYW5kc3RpcGVuZGl1bSDigKIgQmV3ZXJidW5nIGFiZ2ViZW4AAAAAAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5L2VkaXQ/c3RlcD0yAGh0dHBzOi8vZHN0aXAuenV2LnR1LWJlcmxpbi5kZS9zdHVkZW50L2FwcGxpY2F0aW9ucy8zNTM5P3N0ZXA9MQBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0AAAAAAAAk/PGD9VUBQCS88YP1VQFAAAAAADQAwAAAAAAAAAA8D/oxMTEw4VFAAAAAAAnATQAsQMAAAAAAABEZXV0c2NobGFuZHN0aXBlbmRpdW0g4oCiIEJld2VyYnVuZyBhYmdlYmVuAAAAAABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0P3N0ZXA9MwBodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOT9zdGVwPTIAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvYXBwbGljYXRpb25zLzM1MzkvZWRpdD9zdGVwPTIAAAAAAAAAlfPGD9VUBQCU88YP1VQFAAAAAADQAAAAAAAAAAAA8D/wy8vLyoVFAAAAAAAvATQAswMAAAAAAABEZXV0c2NobGFuZHN0aXBlbmRpdW0g4oCiIEJld2VyYnVuZyBhYmdlYmVuAAAAAABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOS9lZGl0P3N0ZXA9NABodHRwczovL2RzdGlwLnp1di50dS1iZXJsaW4uZGUvc3R1ZGVudC9hcHBsaWNhdGlvbnMvMzUzOT9zdGVwPTMAaHR0cHM6Ly9kc3RpcC56dXYudHUtYmVybGluLmRlL3N0dWRlbnQvYXBwbGljYXRpb25zLzM1MzkvZWRpdD9zdGVwPTMAAAAAAAAAl/PGD9VUBQCW88YP1VQFAAAAAACYAAAAAAAAAAAA8D/wy8vLyoVFAAAAAAAvATQACAEQAkADeASwBQAATAAAAMIF"/>
Comment 1 Michael Catanzaro 2017-09-03 07:24:32 PDT 
Created attachment 319776 [details]
Backtrace from GNOME Bugzilla
Comment 2 Carlos Garcia Campos 2017-09-04 01:24:45 PDT 
This is tricky because the bug was actually when encoding the session state, but it's not easy to know why current index was encoded as 75 when the bf list had 5 items. It's easy to work around when decoding, and we should do it anyway, we don't want any malicious (or just corrupted) session state file to crash webkit.
Comment 3 Carlos Garcia Campos 2017-09-04 01:43:57 PDT 
Created attachment 319845 [details]
Patch
Comment 4 Carlos Garcia Campos 2017-09-04 23:59:19 PDT 
This needs a WebKit2 owner approval
Comment 5 Alex Christensen 2017-09-06 10:38:33 PDT 
Comment on attachment 319845 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319845&action=review

> Source/WebKit/ChangeLog:11
> +        Ensure the current index provided by the session state is not out of actual item list bounds. This is a bug in
> +        the session state decoder, but WebBackForwardList::backForwardListState() is already doing the check and using
> +        the last item index instead, so it's not easy to know where the actual problem is. But in any case we should
> +        still protect the decoder.

If this is a problem with session state decoding, we should fix it in session state decoding.  I think this is the wrong place to add this check.
Comment 6 Carlos Garcia Campos 2017-09-06 10:43:49 PDT 
Ok, I'll move the fix to the glib parser then.
Comment 7 Carlos Garcia Campos 2017-09-07 05:09:45 PDT 
Created attachment 320109 [details]
Patch
Comment 8 Build Bot 2017-09-07 05:10:58 PDT 
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 9 Carlos Garcia Campos 2017-09-07 23:31:32 PDT 
Committed r221779: <http://trac.webkit.org/changeset/221779>