Bug 130894 – ASSERTION FAILED: url.containsOnlyASCII() in WebCore::checkEncodedString

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebCore Misc.

[Renata Hodovan]

Created attachment 228042 [details]
Test case

The failing test:

<em>
	<div>
		<iframe src="http://¼"></iframe>
</em>


The backtrace:

ASSERTION FAILED: url.containsOnlyASCII()
/home/reni2/data/REPOS/webkit_sec/Source/WebCore/platform/URL.cpp(303) : void WebCore::checkEncodedString(const WTF::String&)
1   0x7ffff5ed9db5 WTFCrash
2   0x7ffff1592b32
3   0x7ffff1595a26 WebCore::URL::parse(WTF::String const&)
4   0x7ffff1592c54 WebCore::URL::URL(WebCore::ParsedURLStringTag, WTF::String const&)
5   0x7ffff13f43ba WebCore::NavigationScheduler::scheduleLocationChange(WebCore::SecurityOrigin*, WTF::String const&, WTF::String const&, bool, bool)
6   0x7ffff140eb19 WebCore::SubframeLoader::loadOrRedirectSubframe(WebCore::HTMLFrameOwnerElement&, WebCore::URL const&, WTF::AtomicString const&, bool, bool)
7   0x7ffff140d83e WebCore::SubframeLoader::requestFrame(WebCore::HTMLFrameOwnerElement&, WTF::String const&, WTF::AtomicString const&, bool, bool)
8   0x7ffff116df0f WebCore::HTMLFrameElementBase::openURL(bool, bool)
9   0x7ffff116e2c6 WebCore::HTMLFrameElementBase::setNameAndOpenURL()
10  0x7ffff116e397 WebCore::HTMLFrameElementBase::didNotifySubtreeInsertions(WebCore::ContainerNode*)
11  0x7ffff0f35198 WebCore::ChildNodeInsertionNotifier::notify(WebCore::Node&)
12  0x7ffff0f32b96 WebCore::ContainerNode::parserAppendChild(WTF::PassRefPtr<WebCore::Node>)
13  0x7ffff125a196
14  0x7ffff125a34c
15  0x7ffff125a403
16  0x7ffff125a71a WebCore::HTMLConstructionSite::executeQueuedTasks()
17  0x7ffff1287a52 WebCore::HTMLTreeBuilder::constructTree(WebCore::AtomicHTMLToken*)
18  0x7ffff1262ba8 WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLToken&)
19  0x7ffff126282f WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
20  0x7ffff1262035 WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
21  0x7ffff12630ef WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>)
22  0x7ffff0f47d7b WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&)
23  0x7ffff13c55ab WebCore::DocumentWriter::end()
24  0x7ffff13aff8b WebCore::DocumentLoader::finishedLoading(double)
25  0x7ffff13afcf4 WebCore::DocumentLoader::notifyFinished(WebCore::CachedResource*)
26  0x7ffff1458bfc WebCore::CachedResource::checkNotify()
27  0x7ffff1458cda WebCore::CachedResource::finishLoading(WebCore::ResourceBuffer*)
28  0x7ffff14556ae WebCore::CachedRawResource::finishLoading(WebCore::ResourceBuffer*)
29  0x7ffff141095e WebCore::SubresourceLoader::didFinishLoading(double)
30  0x7ffff140cc19 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double)
31  0x7ffff1cf6715
ERR<5518>:eet eet_lib.c:668 eet_shutdown() File '/home/reni2/.cache/efreet/icon_themes_reni2.eet' is still open !

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff5ed9dba in WTFCrash () at /home/reni2/data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:333
333	    *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0x00007ffff5ed9dba in WTFCrash () at /home/reni2/data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:333
#1  0x00007ffff1592b32 in WebCore::checkEncodedString (url=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/platform/URL.cpp:303
#2  0x00007ffff1595a26 in WebCore::URL::parse (this=0x7fffffffba60, string=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/platform/URL.cpp:982
#3  0x00007ffff1592c54 in WebCore::URL::URL (this=0x7fffffffba60, url=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/platform/URL.cpp:330
#4  0x00007ffff13f43ba in WebCore::NavigationScheduler::scheduleLocationChange (this=0x809cb8, securityOrigin=0x999af0, url=..., referrer=..., 
    lockHistory=true, lockBackForwardList=true) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/NavigationScheduler.cpp:357
#5  0x00007ffff140eb19 in WebCore::SubframeLoader::loadOrRedirectSubframe (this=0x905dc0, ownerElement=..., url=..., frameName=..., lockHistory=true, 
    lockBackForwardList=true) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/SubframeLoader.cpp:327
#6  0x00007ffff140d83e in WebCore::SubframeLoader::requestFrame (this=0x905dc0, ownerElement=..., urlString=..., frameName=..., lockHistory=true, 
    lockBackForwardList=true) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/SubframeLoader.cpp:90
#7  0x00007ffff116df0f in WebCore::HTMLFrameElementBase::openURL (this=0x75b5c0, lockHistory=true, lockBackForwardList=true)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:88
#8  0x00007ffff116e2c6 in WebCore::HTMLFrameElementBase::setNameAndOpenURL (this=0x75b5c0)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:132
#9  0x00007ffff116e397 in WebCore::HTMLFrameElementBase::didNotifySubtreeInsertions (this=0x75b5c0)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:157
#10 0x00007ffff0f35198 in WebCore::ChildNodeInsertionNotifier::notify (this=0x7fffffffbda0, node=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/dom/ContainerNodeAlgorithms.h:233
#11 0x00007ffff0f32b96 in WebCore::ContainerNode::parserAppendChild (this=0x9922a0, newChild=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/dom/ContainerNode.cpp:744
#12 0x00007ffff125a196 in WebCore::insert (task=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:96
#13 0x00007ffff125a34c in WebCore::executeInsertAlreadyParsedChildTask (task=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:125
#14 0x00007ffff125a403 in WebCore::executeTask (task=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:145
#15 0x00007ffff125a71a in WebCore::HTMLConstructionSite::executeQueuedTasks (this=0x999848)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:191
#16 0x00007ffff1287a52 in WebCore::HTMLTreeBuilder::constructTree (this=0x999830, token=0x7fffffffbf20)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:366
#17 0x00007ffff1262ba8 in WebCore::HTMLDocumentParser::constructTreeFromHTMLToken (this=0x6ff330, rawToken=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:352
#18 0x00007ffff126282f in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x6ff330, mode=WebCore::HTMLDocumentParser::AllowYield)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:309
#19 0x00007ffff1262035 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x6ff330, mode=WebCore::HTMLDocumentParser::AllowYield)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:189
#20 0x00007ffff12630ef in WebCore::HTMLDocumentParser::append (this=0x6ff330, inputSource=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:428
#21 0x00007ffff0f47d7b in WebCore::DecodedDataDocumentParser::flush (this=0x6ff330, writer=...)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/dom/DecodedDataDocumentParser.cpp:60
#22 0x00007ffff13c55ab in WebCore::DocumentWriter::end (this=0x9029e0) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/DocumentWriter.cpp:245
#23 0x00007ffff13aff8b in WebCore::DocumentLoader::finishedLoading (this=0x902940, finishTime=0)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:440
#24 0x00007ffff13afcf4 in WebCore::DocumentLoader::notifyFinished (this=0x902940, resource=0x977320)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:374
#25 0x00007ffff1458bfc in WebCore::CachedResource::checkNotify (this=0x977320)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedResource.cpp:332
#26 0x00007ffff1458cda in WebCore::CachedResource::finishLoading (this=0x977320)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedResource.cpp:348
#27 0x00007ffff14556ae in WebCore::CachedRawResource::finishLoading (this=0x977320, data=0x969700)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedRawResource.cpp:96
#28 0x00007ffff141095e in WebCore::SubresourceLoader::didFinishLoading (this=0x977860, finishTime=0)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/SubresourceLoader.cpp:310
#29 0x00007ffff140cc19 in WebCore::ResourceLoader::didFinishLoading (this=0x977860, finishTime=0)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/loader/ResourceLoader.cpp:508
---Type <return> to continue, or q <return> to quit---
#30 0x00007ffff1cf6715 in WebCore::readCallback (asyncResult=0x8539e0, data=0x977c80)
    at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1324
#31 0x00007fffe8ee102a in async_ready_callback_wrapper (source_object=0x9720c0, res=0x8539e0, user_data=0x977c80) at ginputstream.c:530
#32 0x00007fffe8f005bb in g_task_return_now (task=0x8539e0) at gtask.c:1105
#33 0x00007fffe8f005d9 in complete_in_idle_cb (task=0x8539e0) at gtask.c:1114
#34 0x00007fffed28af46 in g_main_dispatch (context=0x853720) at gmain.c:3054
#35 g_main_context_dispatch (context=context@entry=0x853720) at gmain.c:3630
#36 0x00007ffff78dc6e8 in _ecore_glib_select__locked (ecore_timeout=<optimized out>, efds=<optimized out>, wfds=0x7fffffffc690, rfds=0x7fffffffc610, 
    ecore_fds=10, ctx=<optimized out>) at ecore_glib.c:171
#37 _ecore_glib_select (ecore_fds=10, rfds=0x7fffffffc610, wfds=0x7fffffffc690, efds=<optimized out>, ecore_timeout=<optimized out>) at ecore_glib.c:205
#38 0x00007ffff78d6b37 in _ecore_main_select (timeout=timeout@entry=0) at ecore_main.c:1466
#39 0x00007ffff78d762c in _ecore_main_loop_iterate_internal (once_only=once_only@entry=0) at ecore_main.c:1860
#40 0x00007ffff78d79c7 in ecore_main_loop_begin () at ecore_main.c:956
#41 0x0000000000406866 in main (argc=2, argv=0x7fffffffdb28) at /home/reni2/data/REPOS/webkit_sec/Tools/EWebLauncher/main.c:1002

[Peter Molnar]

Created attachment 228833 [details]
WIP path

Attached a WIP patch.

Instead of the ASSERT, we invalidate the URL if it contains non-ASCII characters.
I don't know if this is the correct way, do you have any thoughts on this?

[Alexey Proskuryakov]

Comment on attachment 228833 [details]
WIP path

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

> Source/WebCore/ChangeLog:8
> +        Instead of the ASSERT, invalidate the URL if it contains non-ASCII characters.

This significantly changes the semantics. The purpose of the ASSERT was to ensure that URL parsing does what it's supposed to do. The new code makes it so that URL parsing bever does anything useful for non-ASCII input. I think that in this case, you need to demonstrate why this is desired behavior, not ask reviewers to investigate on their own.

What do other browsers do on this test, do they parse the URL into something valid?

This change definitely needs regression tests.

> Source/WebCore/platform/URL.cpp:981
> +    ASSERT_UNUSED(string, string.isEmpty() || isSchemeFirstChar(string[0]));

ASSERT_UNUSED is incorrect here, the string argument is used in the function.

[Benjamin Poulain]

(In reply to comment #1)
> Created an attachment (id=228833) [details]
> WIP path
> 
> Attached a WIP patch.
> 
> Instead of the ASSERT, we invalidate the URL if it contains non-ASCII characters.
> I don't know if this is the correct way, do you have any thoughts on this?

Definitely incorrect.

The problem here is someone is using URL(WebCore::ParsedURLStringTag, WTF::String const&) with a String that was is not a parsed URL String. That is very very bad.

[Renata Hodovan]

*** This bug has been marked as a duplicate of bug 132224 ***

[Alexey Proskuryakov]

It seems unlikely that this is a complete duplicate - we can probably exercise the same code path in URL in other ways.

[Renata Hodovan]

(In reply to comment #5)
> It seems unlikely that this is a complete duplicate - we can probably exercise the same code path in URL in other ways.

(In reply to comment #5)
> It seems unlikely that this is a complete duplicate - we can probably exercise the same code path in URL in other ways.

You were right, the assertion fail still can be triggered with a different test case. A new bug is filed for that: https://bugs.webkit.org/show_bug.cgi?id=133846