Bug 59949

Summary: Avoid potential buffer overflow in WTFLog() and WTFLogVerbose()
Product: WebKit Reporter: Jeff Miller <jeffm>
Component: New BugsAssignee: Jeff Miller <jeffm>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Avoid potential buffer underrun in WTFLog() and WTFLogVerbose() ap: review+

Description Jeff Miller 2011-05-02 11:03:01 PDT
Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()
Comment 1 Jeff Miller 2011-05-02 11:07:47 PDT
Created attachment 91941 [details]
Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()
Comment 2 Alexey Proskuryakov 2011-05-02 11:13:59 PDT
Comment on attachment 91941 [details]
Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()

View in context: https://bugs.webkit.org/attachment.cgi?id=91941&action=review

> Source/JavaScriptCore/ChangeLog:5
> +        Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()

I think it's a buffer overrun, not underrun - <http://en.wikipedia.org/wiki/Buffer_underrun>.

> Source/JavaScriptCore/wtf/Assertions.cpp:277
> +    if (!format)
> +        return;

I'm not sure if this is a good time to make this check after calling vprintf_stderr_common. Or if it's really necessary - crashing on null ptr access if fairly safe, even if we were guarding against malicious input, which we aren't really.

> Source/JavaScriptCore/wtf/Assertions.cpp:279
> +    size_t formatLen = strlen(format);

Please don't abbreviate.

> Source/JavaScriptCore/wtf/Assertions.cpp:294
> +    if (!format) {

Same comment about necessity of the check.
Comment 3 Jeff Miller 2011-05-02 11:17:41 PDT
(In reply to comment #2)
> (From update of attachment 91941 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91941&action=review
> 
> > Source/JavaScriptCore/ChangeLog:5
> > +        Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()
> 
> I think it's a buffer overrun, not underrun - <http://en.wikipedia.org/wiki/Buffer_underrun>.

Heh, actually according to that article it's always a buffer overflow when talking about memory.

> 
> > Source/JavaScriptCore/wtf/Assertions.cpp:277
> > +    if (!format)
> > +        return;
> 
> I'm not sure if this is a good time to make this check after calling vprintf_stderr_common. Or if it's really necessary - crashing on null ptr access if fairly safe, even if we were guarding against malicious input, which we aren't really.

OK, I'll remove the check.

> 
> > Source/JavaScriptCore/wtf/Assertions.cpp:279
> > +    size_t formatLen = strlen(format);
> 
> Please don't abbreviate.

Changed to formatLength.

> 
> > Source/JavaScriptCore/wtf/Assertions.cpp:294
> > +    if (!format) {
> 
> Same comment about necessity of the check.

Also removed this check.
Comment 4 Jeff Miller 2011-05-02 11:20:47 PDT
Landed http://trac.webkit.org/changeset/85496