nssutil_escapeQuotesSize returns the wrong size

RESOLVED FIXED in Firefox -esr17


6 years ago
3 years ago


(Reporter: ted, Assigned: ted)


(4 keywords)

crash, csectype-bounds, regression, sec-high

Firefox Tracking Flags

(firefox-esr10 wontfix, firefox-esr17 fixed)



(2 attachments)

nssutil_escapeQuotesSize seems to be broken. Compare the new code:

with the old code in secmod_doubleEscapeSize:

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
Created attachment 654975 [details]

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.
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.


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.

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
Last Resolved: 6 years ago
Resolution: --- → FIXED
status-firefox-esr10: --- → wontfix
status-firefox-esr17: --- → fixed
Keywords: csec-bounds, sec-high
FWIW, I think we could open this up because this never shipped in any release version of NSS or Firefox, AFAIK.
Assignee: rrelyea → ted


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.