Bug 59949 - Avoid potential buffer overflow in WTFLog() and WTFLogVerbose()
Summary: Avoid potential buffer overflow in WTFLog() and WTFLogVerbose()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeff Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-02 11:03 PDT by Jeff Miller
Modified: 2011-05-02 11:20 PDT (History)
1 user (show)

See Also:


Attachments
Avoid potential buffer underrun in WTFLog() and WTFLogVerbose() (1.68 KB, patch)
2011-05-02 11:07 PDT, Jeff Miller
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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