158396 - Buffer overrun in NSPR logging

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

Problem:
export NSPR_LOG_MODULES=`perl -e 'print "A"x10000'`
mozilla
run-mozilla.sh: line 72:  2074 Floating point exception$prog ${1+"$@"}
mozilla -g
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 2093)]
0x41414141 in ?? ()
(gdb) 
0x41 == 'A'

Georgi Guninski

Comment 1

[Mitchell Stoltz (not reading bugmail)]

->NSPR.

Comment 2

[Wan-Teh Chang]

Attachment: Proposed patch

This is an off-by-one error.  We tried to prevent
buffer overrun by specifying a width for the %[]
conversion, but the width we specify is the size
of the buffer, which does not leave any space for
the terminating null byte.

The fix is to specify a width of the size of the
buffer minus one.

I also took the opportunity to use the EOF macro
instead of the constant -1.  This is a code cleanup
and has nothing to do with this bug.

Please review this patch.

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 94268 [details] [diff] [review]
Proposed patch

Is it possible to use the DEFAULT_BUF_SIZE macro as part of the sscanf format
string? That way, if the buffer size is ever changed, the sscanf operation will
pick up the change automatically. If this is not practical, please add a
comment where the buffer size is defined that says "Security-Critical: If you
change this size, you must also change the sscanf format strings to be size-1."

With that change, r=mstoltz.

Comment 4

[Wan-Teh Chang]

Attachment: Proposed patch v2

It's rather tricky to construct the sscanf format strings
from a macro.  (I would need to play with the preprocessor
stringize operator.)  So I just added the comments you
suggested.

Comment 5

[Wan-Teh Chang]

Fix checked into the tip and NSPRPUB_PRE_4_2_CLIENT_BRANCH
of NSPR.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Does NSPR_LOG_MODULES only work in debug builds?

Did this change make it into NS 7.01 (Blackbird)?

Comment 7

[Wan-Teh Chang]

NSPR_LOG_MODULES also works in optimized builds if the
code is compiled with the macro FORCE_PR_LOG defined.
I don't know if any Mozilla code is compiled with
FORCE_PR_LOG defined.

This change is not in NETSCAPE_7_01_RTM_RELEASE.

Comment 8

[Daniel Veditz [:dveditz]]

Removing confidential flag for bugs fixed over a year ago