nssutil_escapeQuotesSize returns the wrong size

RESOLVED FIXED in Firefox -esr17

Status

NSS
Libraries
--
critical
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

(4 keywords)

3.14
3.14
crash, csectype-bounds, regression, sec-high

Firefox Tracking Flags

(firefox-esr10 wontfix, firefox-esr17 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
nssutil_escapeQuotesSize seems to be broken. Compare the new code:
http://mxr.mozilla.org/security/source/security/nss/lib/util/utilpars.c#318

with the old code in secmod_doubleEscapeSize:
http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/pk11wrap/pk11pars.c#749

It's not returning a large enough buffer, which means that NSSUTIL_DoubleEscape winds up writing off the end of the buffer.

I noticed this because I ran a debug build from alder (which has pulled in a pre-release 3.14 trunk snapshot of NSS) on Windows (where the CRT does heap checking).
This causes Firefox to crash on startup due to heap corruption when Ted ran it on a  Windows (debug) build.
Group: core-security
Keywords: crash, regression
OS: Linux → All
Hardware: x86_64 → All
Kai, I think this bug has to be fixed before NSS 3.14 release and before NSS 3.14 can be integrated into mozilla-central, based on Ted's report that this causes Firefox to crash at startup.

I am not sure if it is a security issue, but it is a buffer overflow so I marked it as a security bug just in case.
Bob, it seems like this is caused by the patch for bug 753116, which is assigned to you, and which is still open, so I'm going to defer to you on this.
Assignee: nobody → rrelyea
(Assignee)

Comment 4

6 years ago
Created attachment 654975 [details]
stack

Here's the stack from my startup crash, FWIW. It's a little confusing because the CRT tries to display a MessageBox to show the assertion text, but our event loop re-enters and then we deadlock, but you can see that the heap corruption is detected under the call to PORT_Free_Util from nss_InitModules.
(Assignee)

Comment 5

6 years ago
Created attachment 656019 [details] [diff] [review]
fix nssutil_escapeQuotesSize

This patch (against the alder branch's mercurial repository) fixes the issue for me. It simply makes nssutil_escapeQuotesSize return the same length as secmod_escapeSize.

Updated

6 years ago
Attachment #656019 - Flags: review?(rrelyea)
Target Milestone: --- → 3.14

Comment 6

6 years ago
Comment on attachment 656019 [details] [diff] [review]
fix nssutil_escapeQuotesSize

r+ the patch is both right and necessary.

bob
Attachment #656019 - Flags: review?(rrelyea) → review+

Comment 7

6 years ago
Checking in utilpars.c;
/cvsroot/mozilla/security/nss/lib/util/utilpars.c,v  <--  utilpars.c
new revision: 1.2; previous revision: 1.1
done
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
status-firefox-esr10: --- → wontfix
status-firefox-esr17: --- → fixed
Keywords: csec-bounds, sec-high
(Assignee)

Comment 8

5 years ago
FWIW, I think we could open this up because this never shipped in any release version of NSS or Firefox, AFAIK.
Assignee: rrelyea → ted

Updated

3 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.