Closed Bug 1366595 (CVE-2017-7756) Opened 8 years ago Closed 8 years ago

Stack-use-after-scope in NS_strlen while logging errors in XHR

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 54+ fixed
firefox53 --- wontfix
firefox54 + fixed
firefox55 + fixed

People

(Reporter: inferno, Assigned: baku)

References

Details

(5 keywords, Whiteboard: [post-critsmash-triage][adv-main54+][adv-esr52.2+])

Attachments

(2 files)

Testcase
123 bytes, text/html
Details
patch
1.12 KB, patch
qdot
: review+
abillings
: approval-mozilla-beta+
abillings
: approval-mozilla-esr52+
abillings
: sec-approval+
Details | Diff | Splinter Review
Attached file Testcase —
==10811==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fff25529080 at pc 0x7faccf02f971 bp 0x7fff25527800 sp 0x7fff255277f8 READ of size 2 at 0x7fff25529080 thread T0 (Web Content) SCARINESS: 14 (2-byte-read-stack-use-after-scope) #0 0x7faccf02f970 in NS_strlen(char16_t const*) xpcom/base/nsCRTGlue.cpp:83:23 #1 0x7faccf00d50a in cvt_S(SprintfStateStr*, char16_t const*, int, int, int) xpcom/string/nsTextFormatter.cpp:522:17 #2 0x7faccf0095c1 in dosprintf(SprintfStateStr*, char16_t const*, __va_list_tag*) xpcom/string/nsTextFormatter.cpp:1139:14 #3 0x7faccf00655e in vsmprintf xpcom/string/nsTextFormatter.cpp:1307:8 #4 0x7faccf00655e in nsTextFormatter::smprintf(char16_t const*, ...) xpcom/string/nsTextFormatter.cpp:1265 #5 0x7faccf25fcae in nsStringBundle::FormatString(char16_t const*, char16_t const**, unsigned int, char16_t**) intl/strres/nsStringBundle.cpp:340:5 #6 0x7faccf25fa63 in nsStringBundle::FormatStringFromName(char16_t const*, char16_t const**, unsigned int, char16_t**) intl/strres/nsStringBundle.cpp:188:10 #7 0x7facd1aec35d in nsContentUtils::FormatLocalizedString(nsContentUtils::PropertiesFile, char const*, char16_t const**, unsigned int, nsXPIDLString&) dom/base/nsContentUtils.cpp:3867:18 #8 0x7facd1aecf99 in nsContentUtils::ReportToConsole(unsigned int, nsACString const&, nsIDocument const*, nsContentUtils::PropertiesFile, char const*, char16_t const**, unsigned int, nsIURI*, nsString const&, unsigned int, unsigned int) dom/base/nsContentUtils.cpp:3932:10 #9 0x7facd5ad43a5 in LogMessage dom/xhr/XMLHttpRequestMainThread.cpp:446:3 #10 0x7facd5ad43a5 in mozilla::dom::XMLHttpRequestMainThread::SetRequestHeader(nsACString const&, nsACString const&) dom/xhr/XMLHttpRequestMainThread.cpp:3115 #11 0x7facd5aef3dd in mozilla::dom::XMLHttpRequestMainThread::SetRequestHeader(nsACString const&, nsACString const&, mozilla::ErrorResult&) dom/xhr/XMLHttpRequestMainThread.h:274:11 #12 0x7facd3482163 in mozilla::dom::XMLHttpRequestBinding::setRequestHeader(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XMLHttpRequest*, JSJitMethodCallArgs const&) objdir-ff-asan/dom/bindings/XMLHttpRequestBinding.cpp:461:9 #13 0x7facd3c97445 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp:2954:13 #14 0x7facd9daf3a0 in CallJSNative js/src/jscntxtinlines.h:293:15 #15 0x7facd9daf3a0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:470 #16 0x7facd9d8c0dd in CallFromStack js/src/vm/Interpreter.cpp:521:12 #17 0x7facd9d8c0dd in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3028 #18 0x7facd9d79488 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:410:12 #19 0x7facd9db1a99 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:699:15 #20 0x7facd9db2332 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:731:12 #21 0x7facda7e28a9 in Evaluate(JSContext*, js::ScopeKind, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4633:19 #22 0x7facda7e3600 in Evaluate js/src/jsapi.cpp:4658:12 #23 0x7facda7e3600 in JS::Evaluate(JSContext*, JS::AutoObjectVector&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4716 #24 0x7facd2011f52 in nsJSUtils::ExecutionContext::CompileAndExec(JS::CompileOptions&, JS::SourceBufferHolder&) dom/base/nsJSUtils.cpp:232:8 #25 0x7facd5b2b722 in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:2035:23 #26 0x7facd5b27211 in mozilla::dom::ScriptLoader::ProcessRequest(mozilla::dom::ScriptLoadRequest*) dom/script/ScriptLoader.cpp:1753:10 #27 0x7facd5b0dc57 in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) dom/script/ScriptLoader.cpp:1455:10 #28 0x7facd5b0a13e in mozilla::dom::ScriptElement::MaybeProcessScript() dom/script/ScriptElement.cpp:149:18 #29 0x7facd10174e0 in AttemptToExecute objdir-ff-asan/dist/include/nsIScriptElement.h:225:18 #30 0x7facd10174e0 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) parser/html/nsHtml5TreeOpExecutor.cpp:698 #31 0x7facd10105c5 in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:499:7 #32 0x7facd101a68b in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:129:20 #33 0x7faccf17fe74 in mozilla::SchedulerGroup::Runnable::Run() xpcom/threads/SchedulerGroup.cpp:365:25 #34 0x7faccf1af789 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1302:14 #35 0x7faccf1bb2b0 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:393:10 #36 0x7faccffeb741 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:96:21 #37 0x7faccff41cbf in RunInternal ipc/chromium/src/base/message_loop.cc:238:10 #38 0x7faccff41cbf in RunHandler ipc/chromium/src/base/message_loop.cc:231 #39 0x7faccff41cbf in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:211 #40 0x7facd5c4670a in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:156:27 #41 0x7facd98296fb in XRE_RunAppShell() toolkit/xre/nsEmbedFunctions.cpp:893:22 #42 0x7faccff41cbf in RunInternal ipc/chromium/src/base/message_loop.cc:238:10 #43 0x7faccff41cbf in RunHandler ipc/chromium/src/base/message_loop.cc:231 #44 0x7faccff41cbf in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:211 #45 0x7facd982918d in XRE_InitChildProcess(int, char**, XREChildData const*) toolkit/xre/nsEmbedFunctions.cpp:709:34 #46 0x5250c2 in content_process_main ipc/contentproc/plugin-container.cpp:64:30 #47 0x5250c2 in main browser/app/nsBrowserApp.cpp:285 #48 0x7face9ebaf44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287 Address 0x7fff25529080 is located in stack of thread T0 (Web Content) at offset 288 in frame #0 0x7facd5ad3dcf in mozilla::dom::XMLHttpRequestMainThread::SetRequestHeader(nsACString const&, nsACString const&) dom/xhr/XMLHttpRequestMainThread.cpp:3090 This frame has 5 object(s): [32, 40) 'doc.i' (line 442) [64, 80) 'ref.tmp.i' (line 446) [96, 192) 'value' (line 3102) [224, 232) 'params' (line 3114) [256, 416) 'ref.tmp' (line 3114) <== Memory access at offset 288 is inside this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-use-after-scope (/mnt/scratch0/clusterfuzz/slave-bot/builds/linux_asan_firefox/custom/firefox/libxul.so+0x1f70970) Shadow bytes around the buggy address: 0x100064a9d1c0: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 f3 f3 0x100064a9d1d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100064a9d1e0: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 0x100064a9d1f0: 00 f2 f2 f2 00 00 f2 f2 00 00 00 00 00 00 00 00 0x100064a9d200: 00 00 00 00 f2 f2 f2 f2 00 f2 f2 f2 f8 f8 f8 f8 =>0x100064a9d210:[f8]f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 0x100064a9d220: f3 f3 f3 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00 0x100064a9d230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100064a9d240: f1 f1 f1 f1 00 00 f2 f2 00 00 f2 f2 00 00 f3 f3 0x100064a9d250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x100064a9d260: 00 00 00 00 f1 f1 f1 f1 00 00 f2 f2 00 00 00 f2 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==10811==ABORTING
XHR isn't allowed to set headers that start with sec-, triggering this logging: 3114 const char16_t* params[] = { NS_ConvertUTF8toUTF16(aName).get() }; 3115 LogMessage("ForbiddenHeaderWarning", GetOwner(), params, ArrayLength(params)); https://searchfox.org/mozilla-central/source/dom/xhr/XMLHttpRequestMainThread.cpp#3110
Component: General → DOM: Core & HTML
Flags: needinfo?(amarchesini)
Summary: Stack-use-after-scope in NS_strlen → Stack-use-after-scope in NS_strlen while logging errors in XHR
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attached patch patch — — Splinter Review
Attachment #8870401 - Flags: review?(kyle)
This seems like a reasonably easy-to-trigger bug that would involve not only stack use-after-scope, but also trigger UAF. sec-high? Trying to figure out whether we'd want to uplift to 54 or not.
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → affected
Flags: needinfo?(continuation)
Actually, just requesting tracking anyway, since this is a pretty straightforward, low-risk fix.
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
Flags: needinfo?(continuation)
Keywords: csectype-uaf, sec-high
[Tracking Requested - why for this release]: security bugs are a good thing to fix for ESR.
tracking-firefox-esr52: --- → ?
Attachment #8870401 - Flags: review?(kyle) → review+
Comment on attachment 8870401 [details] [diff] [review] patch Approval Request Comment [Feature/Bug causing the regression]: bug 1295128 [User impact if declined]: UAF [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: I need an approval first. [Needs manual test from QE? If yes, steps to reproduce]: yes, with an ASAN build. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: the fix is trivial: the char* is taken from a string in memory. [String changes made/needed]: none [Security approval request comment] How easily could an exploit be constructed based on the patch? It's a UAF bug. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? All. patch of 1295128 landed 16 of August, 2016. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? easy to do. How likely is this patch to cause regressions; how much testing does it need? none.
Attachment #8870401 - Flags: sec-approval?
Attachment #8870401 - Flags: approval-mozilla-esr52?
Attachment #8870401 - Flags: approval-mozilla-beta?
Track 54+/55+ as sec-high.
tracking-firefox54: ? → +
tracking-firefox55: ? → +
sec-approval+ for trunk. We'll want to take this everywhere.
status-firefox53: affected → wontfix
tracking-firefox-esr52: ? → 54+
Attachment #8870401 - Flags: sec-approval?
Attachment #8870401 - Flags: sec-approval+
Attachment #8870401 - Flags: approval-mozilla-esr52?
Attachment #8870401 - Flags: approval-mozilla-esr52+
Attachment #8870401 - Flags: approval-mozilla-beta?
Attachment #8870401 - Flags: approval-mozilla-beta+
Blocks: 1295128
Keywords: regression, testcase
https://hg.mozilla.org/mozilla-central/rev/f0ade763cf1e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: core-security → core-security-release
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main54+][adv-esr52.2+]
Alias: CVE-2017-7756
I was unable to reproduce the crash on various presumably affected asan builds using the test case from Comment 0, with and without the fuzzPriv extension, on Ubuntu 16.04 x64. Here's what I've used: * 55.0a1 linux64-asan from 2017-04-10 (20170410233251) * 55.0a1 linux64-asan from 2017-05-09 (20170509120348) * 55.0a1 linux64-asan from 2017-05-21 (20170521163721) Since I couldn't reproduce the crash, I can't confirm if this is fixed or not. Abhishek Arya, could you please verify this on the latest asan builds of 54.0b [1], 55.0a1 [2] and 52.2.0esr [3]? [1] https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-beta.pushdate.2017.06.08.20170608174002.firefox/gecko.v2.mozilla-beta.pushdate.2017.06.08.20170608174002.firefox [2] https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-central.pushdate.2017.06.09.20170609013809.firefox/gecko.v2.mozilla-central.pushdate.2017.06.09.20170609013809.firefox [3] https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-esr52.pushdate.2017.06.08.20170608175922.firefox/gecko.v2.mozilla-esr52.pushdate.2017.06.08.20170608175922.firefox
Flags: qe-verify+ → needinfo?(inferno)
I only build on trunk and can verify in ASan build that it is fixed.
Flags: needinfo?(inferno)
See Also: → 1374024
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: