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

RESOLVED FIXED in Thunderbird 58.0

Status

MailNews Core
Composition
--
critical
RESOLVED FIXED
a month ago
a day ago

People

(Reporter: BenB, Assigned: Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?])

Tracking

(Blocks: 1 bug, {crash})

Thunderbird 58.0
crash

Thunderbird Tracking Flags

(thunderbird_esr52 fixed, thunderbird57 fixed, thunderbird58 fixed)

Details

(Whiteboard: TB 57 beta => TB 52.5 ESR)

Attachments

(1 attachment)

1.10 KB, patch
aceman
: review+
Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?]
: approval-comm-beta+
Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?]
: approval-comm-esr52+
Details | Diff | Splinter Review
(Reporter)

Description

a month ago
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.
(Reporter)

Comment 1

a month ago
For the original report as PDF; see bug 1411701.

Not a security bug.
Blocks: 1411701
Severity: normal → critical
Created attachment 8922686 [details] [diff] [review]
1411734-draft-info-receipt.patch (v1)
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8922686 - Flags: review?(acelists)

Comment 3

28 days ago
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.
Attachment #8922686 - Flags: review?(acelists) → review+

Comment 4

28 days ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/f24014af19e1
don't access receipt field from draft info if it's not there. r=aceman
Status: ASSIGNED → RESOLVED
Last Resolved: 28 days ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
Attachment #8922686 - Flags: approval-comm-esr52+
Attachment #8922686 - Flags: approval-comm-beta+

Comment 5

26 days ago
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
Summary: Crash via invalid X-Mozilla-Draft-Info header → TBE-01-010: Crash via invalid X-Mozilla-Draft-Info header (DOS)
Beta (TB 57):
https://hg.mozilla.org/releases/comm-beta/rev/57610e579accf11ebc24f0e2121f4d23e1d3039a
status-thunderbird52: --- → affected
status-thunderbird57: --- → fixed
status-thunderbird58: --- → fixed
Whiteboard: TB 57 beta => TB 52.5 ESR
TB 52.5 ESR (should be tracking 57+):
https://hg.mozilla.org/releases/comm-esr52/rev/d194d0a89581
status-thunderbird52: affected → fixed
status-thunderbird52: fixed → ---
status-thunderbird_esr52: --- → fixed
You need to log in before you can comment on or make changes to this bug.