Last Comment Bug 827070 - (CVE-2013-0782) Heap-buffer-overflow WRITE in nsSaveAsCharset::DoCharsetConversion
(CVE-2013-0782)
: Heap-buffer-overflow WRITE in nsSaveAsCharset::DoCharsetConversion
Status: RESOLVED FIXED
[asan][adv-main19+][adv-esr1703+]
: crash, sec-moderate, testcase
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: unspecified
: All All
: -- critical (vote)
: mozilla21
Assigned To: Mats Palmgren (vacation)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-05 18:39 PST by Abhishek Arya
Modified: 2014-07-16 11:27 PDT (History)
9 users (show)
dveditz: sec‑bounty-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
verified
+
verified
-
wontfix
19+
verified
19+
fixed


Attachments
Testcase (760 bytes, text/html)
2013-01-05 18:39 PST, Abhishek Arya
no flags Details
fix (3.95 KB, patch)
2013-01-06 09:11 PST, Mats Palmgren (vacation)
smontagu: review+
bajaj.bhavana: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
bajaj.bhavana: approval‑mozilla‑esr17+
bajaj.bhavana: approval‑mozilla‑b2g18+
abillings: sec‑approval+
Details | Diff | Splinter Review

Description Abhishek Arya 2013-01-05 18:39:05 PST
Created attachment 698362 [details]
Testcase

>==26942== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f83dca1bf36 at pc 0x7f840030e916 bp 0x7fffae650e90 sp 0x7fffae650e88
>WRITE of size 1 at 0x7f83dca1bf36 thread T0
>    #0 0x7f840030e915 in nsSaveAsCharset::DoCharsetConversion(unsigned short const*, char**) src/intl/unicharutil/src/nsSaveAsCharset.cpp:193
>    #1 0x7f840030b29e in nsSaveAsCharset::Convert(unsigned short const*, char**) src/intl/unicharutil/src/nsSaveAsCharset.cpp:103
>    #2 0x7f8403c06bcb in nsEncodingFormSubmission::EncodeVal(nsAString_internal const&, nsCString&, bool) src/content/html/content/src/nsFormSubmission.cpp:734
>    #3 0x7f8403c00320 in nsFSURLEncoded::URLEncode(nsAString_internal const&, nsCString&) src/content/html/content/src/nsFormSubmission.cpp:365
>    #4 0x7f8403bff468 in nsFSURLEncoded::AddNameValuePair(nsAString_internal const&, nsAString_internal const&) src/content/html/content/src/nsFormSubmission.cpp:130
>    #5 0x7f840480efe5 in nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission*) src/content/html/content/src/nsHTMLTextAreaElement.cpp:1052
>    #6 0x7f840480f329 in non-virtual thunk to nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission*) src/content/html/content/src/nsHTMLTextAreaElement.cpp:1053
>    #7 0x7f8403ee7344 in nsHTMLFormElement::WalkFormElements(nsFormSubmission*) src/content/html/content/src/nsHTMLFormElement.cpp:998
>    #8 0x7f8403ee2fbc in nsHTMLFormElement::BuildSubmission(nsFormSubmission**, nsEvent*) src/content/html/content/src/nsHTMLFormElement.cpp:779
>    #9 0x7f8403ee1c16 in nsHTMLFormElement::DoSubmit(nsEvent*) src/content/html/content/src/nsHTMLFormElement.cpp:713
>    #10 0x7f8403edaf47 in nsHTMLFormElement::DoSubmitOrReset(nsEvent*, int) src/content/html/content/src/nsHTMLFormElement.cpp:664
>    #11 0x7f8403eda6e1 in nsHTMLFormElement::Submit() src/content/html/content/src/nsHTMLFormElement.cpp:396
>    #12 0x7f8403edb1b3 in non-virtual thunk to nsHTMLFormElement::Submit() src/content/html/content/src/nsHTMLFormElement.cpp:398
>    #13 0x7f8408bace47 in nsIDOMHTMLFormElement_Submit(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:4656
>    #14 0x7f8415f7e665 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:373
>    #15 0x7f8415f7e665 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:391
>    #16 0x7f8415f2a081 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2367
>    #17 0x7f8415e89bab in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) src/js/src/jsinterp.cpp:348
>    #18 0x7f8415f8be85 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) src/js/src/jsinterp.cpp:537
>    #19 0x7f8415f8da25 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) src/js/src/jsinterp.cpp:576
>    #20 0x7f84156e531e in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) src/js/src/jsapi.cpp:5621
>    #21 0x7f84052ca270 in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) src/dom/base/nsJSEnvironment.cpp:1525
>    #22 0x7f8402f0b0d3 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&) src/content/base/src/nsScriptLoader.cpp:859
>    #23 0x7f8402f085fe in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*) src/content/base/src/nsScriptLoader.cpp:757
>    #24 0x7f8402f034d4 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) src/content/base/src/nsScriptLoader.cpp:705
>    #25 0x7f8402ef4226 in nsScriptElement::MaybeProcessScript() src/content/base/src/nsScriptElement.cpp:139
>    #26 0x7f8404b115a8 in nsIScriptElement::AttemptToExecute() src/../../dist/include/nsIScriptElement.h:220
>    #27 0x7f84068777d8 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) src/parser/html/nsHtml5TreeOpExecutor.cpp:791
>    #28 0x7f840687467f in nsHtml5TreeOpExecutor::RunFlushLoop() src/parser/html/nsHtml5TreeOpExecutor.cpp:595
>    #29 0x7f84068b21fd in nsHtml5ExecutorFlusher::Run() src/parser/html/nsHtml5StreamParser.cpp:127
>    #30 0x7f840e44dc3f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
>    #31 0x7f840e0c2495 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:237
>    #32 0x7f840baf8adc in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
>    #33 0x7f840e73f5f2 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:215
>    #34 0x7f840e73f429 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:208
>    #35 0x7f840e73f2fe in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
>    #36 0x7f840aeeb317 in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
>    #37 0x7f8409a17575 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:288
>    #38 0x7f83fec61284 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3823
>    #39 0x7f83fec66e6a in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
>    #40 0x7f83fec69c30 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4088
>    #41 0x41dad6 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
>    #42 0x41b2f0 in main src/browser/app/nsBrowserApp.cpp:279
>    #43 0x7f842023976c in
>0x7f83dca1bf36 is located 0 bytes to the right of 694-byte region [0x7f83dca1bc80,0x7f83dca1bf36)
>allocated by thread T0 here:
>    #0 0x40fd17 in __interceptor_realloc
>    #1 0x7f841f48b208 in PR_Realloc src/nsprpub/pr/src/malloc/prmem.c:450
>    #2 0x7f840030cb59 in nsSaveAsCharset::HandleFallBack(unsigned int, char**, int*, int*, int) src/intl/unicharutil/src/nsSaveAsCharset.cpp:147
>    #3 0x7f840030fdd2 in nsSaveAsCharset::DoCharsetConversion(unsigned short const*, char**) src/intl/unicharutil/src/nsSaveAsCharset.cpp:227
>    #4 0x7f840030b29e in nsSaveAsCharset::Convert(unsigned short const*, char**) src/intl/unicharutil/src/nsSaveAsCharset.cpp:103
>    #5 0x7f8403c06bcb in nsEncodingFormSubmission::EncodeVal(nsAString_internal const&, nsCString&, bool) src/content/html/content/src/nsFormSubmission.cpp:734
>    #6 0x7f8403c00320 in nsFSURLEncoded::URLEncode(nsAString_internal const&, nsCString&) src/content/html/content/src/nsFormSubmission.cpp:365
>    #7 0x7f8403bff468 in nsFSURLEncoded::AddNameValuePair(nsAString_internal const&, nsAString_internal const&) src/content/html/content/src/nsFormSubmission.cpp:130
>    #8 0x7f840480efe5 in nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission*) src/content/html/content/src/nsHTMLTextAreaElement.cpp:1052
>    #9 0x7f840480f329 in non-virtual thunk to nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission*) src/content/html/content/src/nsHTMLTextAreaElement.cpp:1053
>    #10 0x7f8403ee7344 in nsHTMLFormElement::WalkFormElements(nsFormSubmission*) src/content/html/content/src/nsHTMLFormElement.cpp:998
>    #11 0x7f8403ee2fbc in nsHTMLFormElement::BuildSubmission(nsFormSubmission**, nsEvent*) src/content/html/content/src/nsHTMLFormElement.cpp:779
>    #12 0x7f8403ee1c16 in nsHTMLFormElement::DoSubmit(nsEvent*) src/content/html/content/src/nsHTMLFormElement.cpp:713
>    #13 0x7f8403edaf47 in nsHTMLFormElement::DoSubmitOrReset(nsEvent*, int) src/content/html/content/src/nsHTMLFormElement.cpp:664
>    #14 0x7f8403eda6e1 in nsHTMLFormElement::Submit() src/content/html/content/src/nsHTMLFormElement.cpp:396
>    #15 0x7f8403edb1b3 in non-virtual thunk to nsHTMLFormElement::Submit() src/content/html/content/src/nsHTMLFormElement.cpp:398
>    #16 0x7f8408bace47 in nsIDOMHTMLFormElement_Submit(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:4656
>    #17 0x7f8415f7e665 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:373
>    #18 0x7f8415f7e665 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:391
>    #19 0x7f8415f2a081 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2367
>    #20 0x7f8415e89bab in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) src/js/src/jsinterp.cpp:348
>    #21 0x7f8415f8be85 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) src/js/src/jsinterp.cpp:537
>    #22 0x7f8415f8da25 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) src/js/src/jsinterp.cpp:576
>    #23 0x7f84156e531e in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) src/js/src/jsapi.cpp:5621
>    #24 0x7f84052ca270 in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) src/dom/base/nsJSEnvironment.cpp:1525
>    #25 0x7f8402f0b0d3 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&) src/content/base/src/nsScriptLoader.cpp:859
>Shadow bytes around the buggy address:
>  0x1ff07b943790: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  0x1ff07b9437a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  0x1ff07b9437b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  0x1ff07b9437c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  0x1ff07b9437d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>=>0x1ff07b9437e0: 00 00 00 00 00 00[06]fb fb fb fb fb fb fb fb fb
>  0x1ff07b9437f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1ff07b943800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1ff07b943810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1ff07b943820: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1ff07b943830: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>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
>  Heap righ redzone:     fb
>  Freed Heap region:     fd
>  Stack left redzone:    f1
>  Stack mid redzone:     f2
>  Stack right redzone:   f3
>  Stack partial redzone: f4
>  Stack after return:    f5
>  Stack use after scope: f8
>  Global redzone:        f9
>  Global init order:     f6
>  Poisoned by user:      f7
>  ASan internal:         fe
>Stats: 246M malloced (267M for red zones) by 396769 calls
>Stats: 46M realloced by 23752 calls
>Stats: 213M freed by 267399 calls
>Stats: 79M really freed by 187724 calls
>Stats: 464M (464M-0M) mmaped; 116 maps, 0 unmaps
>  mmaps   by size class: 8:294894; 9:32764; 10:8190; 11:12282; 12:2048; 13:1536; 14:1280; 15:384; 16:1152; 17:1312; 18:48; 19:40; 20:24;
>  mallocs by size class: 8:332060; 9:31374; 10:8623; 11:15688; 12:2433; 13:1698; 14:1587; 15:391; 16:1426; 17:1357; 18:70; 19:40; 20:22;
>  frees   by size class: 8:219567; 9:22423; 10:4961; 11:13499; 12:1444; 13:1211; 14:1418; 15:267; 16:1156; 17:1339; 18:57; 19:38; 20:19;
>  rfrees  by size class: 8:166298; 9:7607; 10:2078; 11:8865; 12:594; 13:509; 14:423; 15:153; 16:968; 17:198; 18:26; 19:4; 20:1;
>Stats: malloc large: 1489 small slow: 2310
>Stats: StackDepot: 0 ids; 0M mapped
>==26942== ABORTING
>
Comment 1 Mats Palmgren (vacation) 2013-01-06 08:47:38 PST
DoCharsetConversion is writing the terminating NUL one byte beyond the buffer end.
Most of this code is from 1999 so it likely affects all versions.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/intl%2Funicharutil%2Fsrc%2FnsSaveAsCharset.cpp#200
Comment 2 Mats Palmgren (vacation) 2013-01-06 09:11:56 PST
Created attachment 698441 [details] [diff] [review]
fix

The fix is to allocate an extra byte (two places) but not include
that byte in 'bufferLength' so that the encoder only fills the
buffer to the allocated length - 1.  That makes the
dstPtr[pos2] = '\0' statements always valid.

Also, make HandleFallBack allocate an extra 512 bytes when it needs
to realloc, just like the original malloc in DoCharsetConversion.
This avoids a few reallocs when several HandleFallBack occurs.

https://tbpl.mozilla.org/?tree=Try&rev=4d35e689e65c
Comment 3 Mats Palmgren (vacation) 2013-01-08 04:36:30 PST
It's an out-of-bounds write so I'm guessing sec-critical.  It's only overwriting
by one character and always writes a zero so could be hard to exploit though.
Comment 4 Mats Palmgren (vacation) 2013-01-08 04:46:45 PST
Comment on attachment 698441 [details] [diff] [review]
fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
don't know

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
It's pretty easy to see that the code changes involves a buffer overflow
on something related to "charset".  The added code comment makes that
somewhat easier to see that we were writing the terminating zero beyond
the buffer end in some cases.

Which older supported branches are affected by this flaw?
all

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I think the same patch would apply to all branches.

How likely is this patch to cause regressions; how much testing does it need?
unlikely; nothing special
Comment 5 Al Billings [:abillings] 2013-01-08 12:56:11 PST
Comment on attachment 698441 [details] [diff] [review]
fix

sec-approval+ to check this in on 1/18 or later.
Comment 7 Phil Ringnalda (:philor, back in August) 2013-01-20 13:59:51 PST
https://hg.mozilla.org/mozilla-central/rev/2106f6631cd6
Comment 8 Daniel Veditz [:dveditz] 2013-01-22 15:46:53 PST
Please work on patches for the other affected branches, ESR-17, b2g18, aurora and beta.
Comment 9 Mats Palmgren (vacation) 2013-01-22 16:49:33 PST
Comment on attachment 698441 [details] [diff] [review]
fix

See comment 4.
This patch applies as is to ESR-17, b2g18, aurora and beta.
Comment 11 Matt Wobensmith [:mwobensmith][:matt:] 2013-01-24 14:12:30 PST
Confirmed crash 2013-01-05, m-c 
Confirmed fixed 2013-01-24, m-c 
Confirmed fixed 2013-01-24, Aurora
Confirmed fixed 2013-01-24, beta
Confirmed fixed 2013-01-24, 17.0.2esr
Comment 12 Daniel Veditz [:dveditz] 2013-01-28 15:18:11 PST
Based on comment 3 lowering the severity to sec-moderate.

Note You need to log in before you can comment on or make changes to this bug.