1883804 - Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Tried to atomize invalid UTF-8.), at /builds/worker/checkouts/gecko/xpcom/ds/nsAtomTable.cpp:556

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20240122-6b4a069fe37d (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Tried to atomize invalid UTF-8.), at /builds/worker/checkouts/gecko/xpcom/ds/nsAtomTable.cpp:556

#0 0x7f1a66849f57 in nsAtomTable::Atomize(nsTSubstring<char> const&) /builds/worker/checkouts/gecko/xpcom/ds/nsAtomTable.cpp:556:5
#1 0x7f1a6684a3bc in NS_Atomize(nsTSubstring<char> const&) /builds/worker/checkouts/gecko/xpcom/ds/nsAtomTable.cpp:590:22
#2 0x7f1a686451ab in mozilla::dom::IDTracker::ResetWithLocalRef(mozilla::dom::Element&, nsTSubstring<char16_t> const&, bool) /builds/worker/checkouts/gecko/dom/base/IDTracker.cpp:141:27
#3 0x7f1a6b2b4746 in UpdateHrefTarget /builds/worker/checkouts/gecko/dom/svg/SVGAnimationElement.cpp:347:17
#4 0x7f1a6b2b4746 in mozilla::dom::SVGAnimationElement::BindToTree(mozilla::dom::BindContext&, nsINode&) /builds/worker/checkouts/gecko/dom/svg/SVGAnimationElement.cpp:149:7
#5 0x7f1a6880e2cb in nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/base/nsINode.cpp:1618:15
#6 0x7f1a67a895a7 in AppendChildTo /builds/worker/checkouts/gecko/dom/base/nsINode.h:978:5
#7 0x7f1a67a895a7 in nsHtml5TreeOperation::Append(nsIContent*, nsIContent*, nsHtml5DocumentBuilder*) /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOperation.cpp:261:12
#8 0x7f1a67a7f0e8 in nsHtml5TreeOperation::Append(nsIContent*, nsIContent*, mozilla::dom::FromParser, nsHtml5DocumentBuilder*) /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOperation.cpp:278:17
#9 0x7f1a67a83e34 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/checkouts/gecko/parser/html/nsHtml5TreeOpExecutor.cpp:695:19
#10 0x7f1a67a8b4b8 in nsHtml5ExecutorFlusher::Run() /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp:173:18
#11 0x7f1a668f3377 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:578:16
#12 0x7f1a668e8ae6 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:905:26
#13 0x7f1a668e72c7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:728:15
#14 0x7f1a668e7745 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:36
#15 0x7f1a668f7316 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:232:37
#16 0x7f1a668f7316 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#17 0x7f1a6690c682 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1199:16
#18 0x7f1a669137cd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#19 0x7f1a675f9065 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#20 0x7f1a6750f411 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#21 0x7f1a6750f411 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#22 0x7f1a6be68158 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#23 0x7f1a6bf2abb8 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:470:33
#24 0x7f1a6dd6282b in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:712:20
#25 0x7f1a675f9f46 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#26 0x7f1a6750f411 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#27 0x7f1a6750f411 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#28 0x7f1a6dd62092 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:647:34
#29 0x559ed9bc53f6 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#30 0x559ed9bc53f6 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
#31 0x7f1a7ba29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#32 0x7f1a7ba29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#33 0x559ed9b9b128 in _start (/home/user/workspace/browsers/m-c-20240304165340-fuzzing-debug/firefox-bin+0x59128) (BuildId: 4cd10e79b03551268afff4f2a04b8043968b0ce3)

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240306044140-099f52f30c39.
The bug appears to have been introduced in the following build range:

Start: cd184ad5b96078dbc9d4251ed366d7acff965d7a (20240101231540)
End: 60c369c930dc96abae06a2e0ad6833a3574b803f (20240102071640)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd184ad5b96078dbc9d4251ed366d7acff965d7a&tochange=60c369c930dc96abae06a2e0ad6833a3574b803f

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Based on comment #1, this bug contains a bisection range found by bugmon. However, the Regressed by field is still not filled.

:tnikkel, since you are the author of the changes in the range, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit BugBot documentation.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1872408

:longsonr, since you are the author of the regressor, bug 1872408, could you take a look?

For more information, please visit BugBot documentation.

Comment 4

[Frederik Braun [:freddy]]

Looks like this should be in SVG and not in xpcom. The code calling NS_Atomize is wrong, not the function itself.
Also, with my release engineering owner hat on: This needs a priority and severity. Given this is a debug assertion and we just end up atomizing a bunch of invalid UTF-8 bytes, I think this is likely fine (let's hope I'm not wrong), I will go ahead and guess this is S3.

Comment 5

[Jonathan Kew [:jfkthame]]

Looks like this comes from "unescaping" the %a8 in the href attribute; the resulting byte 0xA8 is not valid by itself in UTF-8. Probably we just shouldn't attempt to decode any non-ASCII bytes here. Limiting the unescaping to 7-bit ASCII characters only will still address the original issue in bug 1872408, without the risk of ending up with invalid UTF-8 in the string.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1883804 - Only unescape ASCII chars in IDTracker::ResetWithLocalRef. r=longsonr,emilio

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb04b6266691
Only unescape ASCII chars in IDTracker::ResetWithLocalRef. r=longsonr

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1872408

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45202 for changes under testing/web-platform/tests

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/cb04b6266691

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 12

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240319215652-f8b5f6fd2e5a.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox125 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Jonathan Kew [:jfkthame]]

Given how nsAtomTable::Atomize deals with the error (in effect replacing the bad byte(s) with U+FFFD), I guess it's possible this could result in two different href attributes (with different percent-encoded sequences in the non-ASCII range) being mapped to the same atom. Not sure exactly what confusion might follow from that.... So given it's a straightforward fix, probably worth uplifting.

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1883804 - Only unescape ASCII chars in IDTracker::ResetWithLocalRef.

Original Revision: https://phabricator.services.mozilla.com/D205039

Comment 16

[Phabricator Automation]

Uplift Approval Request

• Steps to reproduce for manual QE testing: n/a
• Fix verified in Nightly: no
• Is Android affected?: yes
• Needs manual QE test: no
• String changes made/needed: none
• Code covered by automated testing: yes
• Explanation of risk level: just limits unescaping of %-sequences to ASCII only
• Risk associated with taking this patch: minimal
• User impact if declined: possible incorrect handling of 'href' attribute changes in svg

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/0c809bbdad42