1411734 - TBE-01-010: Crash via invalid X-Mozilla-Draft-Info header (DOS)

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Ben Bucksch (:BenB)]

Thunderbird cannot handle an invalid X-Mozilla-Draft-Info header. Due
to a null pointer dereference, Thunderbird exits with a segmentation fault and must be restarted. The crash happens when the following PoC.eml file is opened in Thunderbird, specifically when the “Edit As New Message” menu option is selected.

PoC.eml:
X-Mozilla-Draft-Info: 1
Content-Type: text/html; charset=utf-8

Please right-click this e-mail and click "Edit As New Message".

The root cause of this issue was found in the following source code.

Affected File:
/mailnews/mime/src/mimedrft.cpp

Affected Code:
draftInfo = MimeHeaders_get(mdd->headers, HEADER_X_MOZILLA_DRAFT_INFO, false,
false);

// Keep the same message id when editing a draft unless we're
// editing a message "as new message" (template) or forwarding inline.
if (mdd->format_out != nsMimeOutput::nsMimeMessageEditorTemplate &&
    fields && !forward_inline) {
  fields->SetMessageId(id);
}

if (draftInfo && fields && !forward_inline)
{
[...]
  parm = MimeHeaders_get_parameter(draftInfo, "receipt", NULL, NULL);
  if (parm && !strcmp(parm, "0"))
    fields->SetReturnReceipt(false);
  else
  {
    int receiptType = 0;
    fields->SetReturnReceipt(true);
    sscanf(parm, "%d", &receiptType);

One can see here that the parm variable is set to the arguments of the draftInfo. Since it requires an argument like “receipt”, it is checked whether that item actually exists.

However, if parm is not set (e.g. it is equal to NULL), it is still being used as a source pointer in a sscanf call, thus causing an invalid access to memory at NULL. It is recommended to make sure that the sscanf code path is not reachable unless parm is set correctly.

Comment 1

[Ben Bucksch (:BenB)]

For the original report as PDF; see bug 1411701.

Not a security bug.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1411734-draft-info-receipt.patch (v1)

Comment 3

[:aceman]

Comment on attachment 8922686 [details] [diff] [review]
1411734-draft-info-receipt.patch (v1)

Review of attachment 8922686 [details] [diff] [review]:
-----------------------------------------------------------------

It could be cleaner if MimeHeaders_get_parameter returned nsCstring (it even uses it inernally). But that seems not to be the trend in the mime files, where everything is plain pointers. Also it would take to change a lot of callers.

Thanks for fixing this.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f24014af19e1
don't access receipt field from draft info if it's not there. r=aceman

Comment 5

[Wayne Mery (:wsmwk)]

tested edit in local drafts
bp-15c0e909-762e-4e07-8955-f32910171029
0	ucrtbase.dll	_invalid_parameter	
1	ucrtbase.dll	_invalid_parameter_noinfo	
2	ucrtbase.dll	_XMMI_FP_Emulation	
3	ucrtbase.dll	__stdio_common_vsscanf	
4	xul.dll	_vsscanf_l	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/vs2015u3/SDK/Include/10.0.14393.0/ucrt/stdio.h:2167
5	xul.dll	sscanf	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/vs2015u3/SDK/Include/10.0.14393.0/ucrt/stdio.h:2265
6	xul.dll	mime_parse_stream_complete	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/mailnews/mime/src/mimedrft.cpp:1272
7	xul.dll	nsStreamConverter::OnStopRequest(nsIRequest*, nsISupports*, nsresult)	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/mailnews/mime/src/nsStreamConverter.cpp:1055
8	xul.dll	nsMsgProtocol::OnStopRequest(nsIRequest*, nsISupports*, nsresult)	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/mailnews/base/util/nsMsgProtocol.cpp:339
9	xul.dll	nsMailboxProtocol::OnStopRequest(nsIRequest*, nsISupports*, nsresult)	C:/builds/moz2_slave/tb-rel-c-esr52-w32_bld-0000000/build/mailnews/local/src/nsMailboxProtocol.cpp:382

spot checking some crash reports of the last 6 months, I find no evidence of this in the wild. not surprising

Comment 6

[Jorg K (CEST = GMT+2)]

Beta (TB 57):
https://hg.mozilla.org/releases/comm-beta/rev/57610e579accf11ebc24f0e2121f4d23e1d3039a

Comment 7

[Jorg K (CEST = GMT+2)]

TB 52.5 ESR (should be tracking 57+):
https://hg.mozilla.org/releases/comm-esr52/rev/d194d0a89581