785208 - nssutil_escapeQuotesSize returns the wrong size

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[(not currently active) Ted Mielczarek]

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

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This causes Firefox to crash on startup due to heap corruption when Ted ran it on a  Windows (debug) build.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 4

[(not currently active) Ted Mielczarek]

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

Comment 5

[(not currently active) Ted Mielczarek]

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

Comment 6

[Robert Relyea]

Comment on attachment 656019 [details] [diff] [review]
fix nssutil_escapeQuotesSize

r+ the patch is both right and necessary.

bob

Comment 7

[Robert Relyea]

Checking in utilpars.c;
/cvsroot/mozilla/security/nss/lib/util/utilpars.c,v  <--  utilpars.c
new revision: 1.2; previous revision: 1.1
done

Comment 8

[(not currently active) Ted Mielczarek]

FWIW, I think we could open this up because this never shipped in any release version of NSS or Firefox, AFAIK.