Bug 59949

Created attachment 91941 [details] Avoid potential buffer underrun in WTFLog() and WTFLogVerbose()

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.

(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.

Landed http://trac.webkit.org/changeset/85496