538939 - Misuse of PR_GetErrorTextLength when allocating error message buffers

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Mike Hommey [:glandium]]

Attachment: patch

This was noticed while playing around with modutil, adding a module which library couldn't dlload()ed, and modutil crashed badly:
*** glibc detected *** ./modutil: free(): invalid next size (fast): 0x0000000001ddfb50 ***
(leaving out the backtrace and memory map)

The problem is that none of the PR_Malloc() calls that depend on PR_GetErrorTextLength in NSS are safe: PR_GetErrorTextLength returns the text length, which means it doesn't include space for the terminating NULL character.
PR_GetErrorText, itself, copies length + 1 characters, so the buffers are under-alloc'ed.

Comment 1

[Wan-Teh Chang]

Comment on attachment 421019 [details] [diff] [review]
patch

r=wtc.  Thanks for the patch.

See my comment in bug 538940 comment 1 for the background
of the off-by-one problem of PR_GetErrorTextLength.

In short, the current implementation of PR_GetErrorTextLength
doesn't match the originally intended specification.  After
I have verified that the current implementation is self-consistent,
I'll change the documentation of PR_GetErrorTextLength to
reflect the current implementation.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Bug 538939: Fix allocation of error message buffers for PR_GetErrorTextLength
Patch contributed by  Mike Hommey <[e-mail]>, r=wtc

Checking in modutil/pk11.c;        new revision: 1.30; previous revision: 1.29
Checking in signtool/javascript.c; new revision: 1.7;  previous revision: 1.6
Checking in signtool/zip.c;        new revision: 1.6;  previous revision: 1.5