RESOLVED FIXED 26462
Out of bounds read / occasional crash (no security consequence) in comment parsing 
https://bugs.webkit.org/show_bug.cgi?id=26462
Summary Out of bounds read / occasional crash (no security consequence) in comment pa...
Chris Evans
Reported 2009-06-16 16:01:36 PDT
Patch based on the following crash triggering several times a day: Thread 1 *CRASHED* (EXCEPTION_ACCESS_VIOLATION @0x01a0b000) 0x01e97f41 [chrome.dll - textresourcedecoder.cpp:621] WebCore::TextResourceDecoder::checkForHeadCharset(char const *,unsigned int,bool &) 0x01e9832f [chrome.dll - textresourcedecoder.cpp:800] WebCore::TextResourceDecoder::decode(char const *,unsigned int) 0x01e42e05 [chrome.dll - frameloader.cpp:1813] WebCore::FrameLoader::addData(char const *,int) 0x02149041 [chrome.dll - webframe_impl.cc:1608] WebFrameImpl::DidReceiveData(WebCore::DocumentLoader *,char const *,int) Something has crashed due to crossing a page boundary. Upon inspection of the code at this location, skipComment() clearly reads past the end of the buffer. I have verified this in the debugger. No reliable test case is available because of the non-deterministic nature of reading out-of-bounds. In fact I have never observed a crash -- but it is clearly happening many times a day in the crash logs. Fix is easy / obvious -- patch upload to follow.
Attachments
Fixes out-of-bounds reads. (1015 bytes, patch)
2009-06-16 16:02 PDT, Chris Evans 		abarth: review-
DetailsFormatted DiffDiff
Fix out-of-bounds reads. (1.89 KB, patch)
2009-06-18 18:49 PDT, Chris Evans 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Evans
Comment 1 2009-06-16 16:02:52 PDT
Created attachment 31390 [details] Fixes out-of-bounds reads.
Adam Barth
Comment 2 2009-06-16 16:09:44 PDT
Comment on attachment 31390 [details] Fixes out-of-bounds reads. Thanks for the patch, but we'll need to include a ChangeLog before landing this.
Darin Adler
Comment 3 2009-06-16 17:12:13 PDT
Also, WebKit coding style has no braces around a single line if body.
Chris Evans
Comment 4 2009-06-18 18:49:51 PDT
Created attachment 31528 [details] Fix out-of-bounds reads.
Chris Evans
Comment 5 2009-06-18 18:50:56 PDT
Request land of patch. Changes since last version: - Adhere to coding style (thanks Darin)! - Add ChangeLog entry. - Includes justification of why I can't reasonably add a test.
Eric Seidel (no email)
Comment 6 2009-06-18 18:57:19 PDT
Comment on attachment 31528 [details] Fix out-of-bounds reads. Tabs in the ChangeLog. Otherwise this looks great. Will have to be landed manually, my script won't be able to fix the tabs (it's not that smart).
Brent Fulgham
Comment 7 2009-06-19 12:48:41 PDT
Assigned for landing.
Brent Fulgham
Comment 8 2009-06-19 12:55:27 PDT
Corrected tabs and landed in http://trac.webkit.org/changeset/44865.
David Kilzer (:ddkilzer)
Comment 9 2009-06-22 17:51:08 PDT
PLEASE add bug numbers to ChangeLog entries in the future.
Chris Evans
Comment 10 2009-06-22 17:55:43 PDT
You got it. Sorry :)
Note You need to log in before you can comment on or make changes to this bug.