Bug 53615 – WebKit2: Restoring session state that contains form data fails (asserts in Debug build)

• Status: RESOLVED
• Resolution: FIXED
• Component: History

[Adam Roben (:aroben)]

To reproduce:

1. Open the attached HTML page
2. Submit the form
2. Save session state
3. Restore session state

The session state can't be restored. In a Debug build, you'll even hit an assertion in Vector::at:

            ASSERT(i < size());

Presumably this could lead to memory corruption, as we then try to memcpy off the end of the Vector's buffer.

Here's the backtrace when the assertion occurs:

>	WebKit.dll!WTF::Vector<unsigned char,0>::at(unsigned int i=0x00000000)  Line 536 + 0x29 bytes	C++
 	WebKit.dll!WTF::Vector<unsigned char,0>::operator[](unsigned int i=0x00000000)  Line 545 + 0x1a bytes	C++
 	WebKit.dll!CoreIPC::ArgumentDecoder::decodeBytes(WTF::Vector<unsigned char,0> & buffer=[0x00000000]())  Line 106 + 0x15 bytes	C++
 	WebKit.dll!WebKit::DecoderAdapter::decodeBytes(WTF::Vector<unsigned char,0> & bytes=[0x00000000]())  Line 41	C++
 	WebKit.dll!WebCore::decode(WTF::Decoder & decoder={...}, WebCore::FormDataElement & element={...})  Line 373 + 0x13 bytes	C++
 	WebKit.dll!WebCore::FormData::decodeForBackForward(WTF::Decoder & decoder={...})  Line 457 + 0xd bytes	C++
 	WebKit.dll!WebCore::HistoryItem::decodeBackForwardTree(const WTF::String & topURLString={file:///C:/Documents%20and%20Settings/Adam%20Roben/dev/test/simple-form.html}, const WTF::String & topTitle={{empty string}}, const WTF::String & topOriginalURLString={file:///C:/Documents%20and%20Settings/Adam%20Roben/dev/test/simple-form.html}, WTF::Decoder & decoder={...})  Line 813 + 0x10 bytes	C++
 	WebKit.dll!WebKit::WebPage::restoreSession(const WebKit::SessionState & sessionState={...})  Line 969 + 0x28 bytes	C++
 	WebKit.dll!WebKit::WebPage::restoreSessionAndNavigateToCurrentItem(const WebKit::SessionState & sessionState={...})  Line 986 + 0xc bytes	C++
 	WebKit.dll!CoreIPC::callMemberFunction<WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(WebKit::SessionState const &),WebKit::SessionState>(const CoreIPC::Arguments1<WebKit::SessionState> & args={...}, WebKit::WebPage * object=0x0ea44e00, void (const WebKit::SessionState &)* function=0x10008f94)  Line 19 + 0xf bytes	C++
 	WebKit.dll!CoreIPC::handleMessage<Messages::WebPage::RestoreSessionAndNavigateToCurrentItem,WebKit::WebPage,void (__thiscall WebKit::WebPage::*)(WebKit::SessionState const &)>(CoreIPC::ArgumentDecoder * argumentDecoder=0x0ea40fd8, WebKit::WebPage * object=0x0ea44e00, void (const WebKit::SessionState &)* function=0x10008f94)  Line 222 + 0x15 bytes	C++
 	WebKit.dll!WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection * __formal=0x04692e00, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x0ea40fd8)  Line 124 + 0x2f bytes	C++
 	WebKit.dll!WebKit::WebPage::didReceiveMessage(CoreIPC::Connection * connection=0x04692e00, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x0ea40fd8)  Line 1676	C++
 	WebKit.dll!WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection * connection=0x04692e00, CoreIPC::MessageID messageID={...}, CoreIPC::ArgumentDecoder * arguments=0x0ea40fd8)  Line 537	C++
 	WebKit.dll!CoreIPC::Connection::dispatchMessages()  Line 441 + 0x31 bytes	C++
 	WebKit.dll!MemberFunctionWorkItem0<CoreIPC::Connection>::execute()  Line 76 + 0x10 bytes	C++
 	WebKit.dll!RunLoop::performWork()  Line 63 + 0x1a bytes	C++
 	WebKit.dll!RunLoop::wndProc(HWND__ * hWnd=0x000d075c, unsigned int message=0x00000401, unsigned int wParam=0x0526afa0, long lParam=0x00000000)  Line 57	C++
 	WebKit.dll!RunLoop::RunLoopWndProc(HWND__ * hWnd=0x000d075c, unsigned int message=0x00000401, unsigned int wParam=0x0526afa0, long lParam=0x00000000)  Line 39 + 0x18 bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x28 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xdc bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	WebKit.dll!RunLoop::run()  Line 73 + 0xc bytes	C++
 	WebKit.dll!WebKit::WebProcessMain(const WebKit::CommandLine & commandLine={...})  Line 82	C++
 	WebKit.dll!WebKitMain(const WebKit::CommandLine & commandLine={...})  Line 48 + 0x9 bytes	C++
 	WebKit.dll!WebKitMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x0002114c, int nCmdShow=0x0000000a)  Line 172 + 0x9 bytes	C++
 	WebKit2WebProcess.exe!wWinMain(HINSTANCE__ * hInstance=0x00400000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x0002114c, int nCmdShow=0x0000000a)  Line 44 + 0x18 bytes	C++
 	WebKit2WebProcess.exe!__tmainCRTStartup()  Line 589 + 0x1c bytes	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23 bytes

[Adam Roben (:aroben)]

Created attachment 80944 [details]
test case

[Adam Roben (:aroben)]

<rdar://problem/8943346>

[Adam Roben (:aroben)]

(In reply to comment #0)
> Presumably this could lead to memory corruption, as we then try to memcpy off the end of the Vector's buffer.

I should note that in my testing I don't actually see memory corruption, as we end up passing a size of 0 to memcpy, so we don't actually write into the bad address.

[Adam Roben (:aroben)]

(In reply to comment #3)
> (In reply to comment #0)
> > Presumably this could lead to memory corruption, as we then try to memcpy off the end of the Vector's buffer.
> 
> I should note that in my testing I don't actually see memory corruption, as we end up passing a size of 0 to memcpy, so we don't actually write into the bad address.

In the implementation of ArgumentDecoder::decodeBytes [1], we decode the number of bytes into a uint64_t, but then pass it to Vector functions and memcpy, which take a size_t. On a 32-bit build on Windows, size_t is only 32 bits wide, so number of bytes gets truncated, in this case leaving us with 0. That's why we pass a size of 0 to memcpy.

This seems like a problem in a number of places in CoreIPC, and should probably be tracked separately. I filed bug 53617 to track this.

1. http://trac.webkit.org/browser/trunk/Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp?rev=77378#L95

[Adam Roben (:aroben)]

Created attachment 80950 [details]
Encode/decode FormData and FormDataElement objects consistently

[Adam Roben (:aroben)]

Committed r77401: <http://trac.webkit.org/changeset/77401>