Buffer overrun in NSPR logging



17 years ago
14 years ago


(Reporter: security-bugs, Assigned: wtc)



Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

export NSPR_LOG_MODULES=`perl -e 'print "A"x10000'`
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 ?? ()
0x41 == 'A'

Georgi Guninski


17 years ago
Assignee: mstoltz → wtc
Component: Security: General → NSPR
Product: Browser → NSPR
QA Contact: bsharma → wtc

Comment 1

17 years ago

Comment 2

17 years ago
Created attachment 94268 [details] [diff] [review]
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

17 years ago
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.
Attachment #94268 - Flags: review+

Comment 4

17 years ago
Created attachment 94574 [details] [diff] [review]
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
Attachment #94268 - Attachment is obsolete: true

Comment 5

17 years ago
Fix checked into the tip and NSPRPUB_PRE_4_2_CLIENT_BRANCH
of NSPR.
Last Resolved: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.3

Comment 6

16 years ago
Does NSPR_LOG_MODULES only work in debug builds?

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

Comment 7

16 years ago
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.
Removing confidential flag for bugs fixed over a year ago
Group: security
You need to log in before you can comment on or make changes to this bug.