59949 2011-05-02 11:03:01 -0700 Avoid potential buffer overflow in WTFLog() and WTFLogVerbose() 2011-05-02 11:20:47 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 jeffm jeffm ap oldest_to_newest 396478 0 jeffm 2011-05-02 11:03:01 -0700 Avoid potential buffer underrun in WTFLog() and WTFLogVerbose() 396483 1 91941 jeffm 2011-05-02 11:07:47 -0700 Created attachment 91941 Avoid potential buffer underrun in WTFLog() and WTFLogVerbose() 396489 2 91941 ap 2011-05-02 11:13:59 -0700 Comment on attachment 91941 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. 396494 3 jeffm 2011-05-02 11:17:41 -0700 (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. 396497 4 jeffm 2011-05-02 11:20:47 -0700 Landed http://trac.webkit.org/changeset/85496 91941 2011-05-02 11:07:47 -0700 2011-05-02 11:13:58 -0700 Avoid potential buffer underrun in WTFLog() and WTFLogVerbose() buffer-underrun.patch text/plain 1724 jeffm SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODU0OTIpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAK KzIwMTEtMDUtMDIgIEplZmYgTWlsbGVyICA8amVmZm1AYXBwbGUuY29tPgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEF2b2lkIHBvdGVudGlhbCBidWZm ZXIgdW5kZXJydW4gaW4gV1RGTG9nKCkgYW5kIFdURkxvZ1ZlcmJvc2UoKQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTk5NDkKKworICAgICAgICAqIHd0 Zi9Bc3NlcnRpb25zLmNwcDogQ2hlY2sgZm9yIDAgb3IgZW1wdHkgZm9ybWF0IHN0cmluZyBpbiBX VEZMb2coKSBhbmQgV1RGTG9nVmVyYm9zZSgpLgorCiAyMDExLTA1LTAyICBBZGFtIEJhcnRoICA8 YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWxleGV5IFByb3NrdXJ5 YWtvdi4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvQXNzZXJ0aW9ucy5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9Bc3NlcnRpb25zLmNwcAkocmV2 aXNpb24gODU0NzUpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0Fzc2VydGlvbnMuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC0yNzIsNyArMjcyLDEyIEBAIHZvaWQgV1RGTG9nKFdURkxvZ0No YW5uZWwqIGNoYW5uZWwsIGNvbnMKICAgICB2YV9zdGFydChhcmdzLCBmb3JtYXQpOwogICAgIHZw cmludGZfc3RkZXJyX2NvbW1vbihmb3JtYXQsIGFyZ3MpOwogICAgIHZhX2VuZChhcmdzKTsKLSAg ICBpZiAoZm9ybWF0W3N0cmxlbihmb3JtYXQpIC0gMV0gIT0gJ1xuJykKKyAgICAKKyAgICBpZiAo IWZvcm1hdCkKKyAgICAgICAgcmV0dXJuOworICAgIAorICAgIHNpemVfdCBmb3JtYXRMZW4gPSBz dHJsZW4oZm9ybWF0KTsKKyAgICBpZiAoZm9ybWF0TGVuICYmIGZvcm1hdFtmb3JtYXRMZW4gLSAx XSAhPSAnXG4nKQogICAgICAgICBwcmludGZfc3RkZXJyX2NvbW1vbigiXG4iKTsKIH0KIApAQCAt Mjg1LDggKzI5MCwxNiBAQCB2b2lkIFdURkxvZ1ZlcmJvc2UoY29uc3QgY2hhciogZmlsZSwgaW50 CiAgICAgdmFfc3RhcnQoYXJncywgZm9ybWF0KTsKICAgICB2cHJpbnRmX3N0ZGVycl9jb21tb24o Zm9ybWF0LCBhcmdzKTsKICAgICB2YV9lbmQoYXJncyk7Ci0gICAgaWYgKGZvcm1hdFtzdHJsZW4o Zm9ybWF0KSAtIDFdICE9ICdcbicpCisKKyAgICBpZiAoIWZvcm1hdCkgeworICAgICAgICBwcmlu dENhbGxTaXRlKGZpbGUsIGxpbmUsIGZ1bmN0aW9uKTsKKyAgICAgICAgcmV0dXJuOworICAgIH0K KyAgICAKKyAgICBzaXplX3QgZm9ybWF0TGVuID0gc3RybGVuKGZvcm1hdCk7CisgICAgaWYgKGZv cm1hdExlbiAmJiBmb3JtYXRbZm9ybWF0TGVuIC0gMV0gIT0gJ1xuJykKICAgICAgICAgcHJpbnRm X3N0ZGVycl9jb21tb24oIlxuIik7CisKICAgICBwcmludENhbGxTaXRlKGZpbGUsIGxpbmUsIGZ1 bmN0aW9uKTsKIH0KIAo=