Bug 22134

Summary: -[WebHistoryItem dictionaryRepresentation] accesses past the end of a vector
Product: WebKit Reporter: Aaron Golden <agolden>
Component: HistoryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
This patch prevents the bad access.
none
Same patch as before, but with a bit more whitespace and a ChangeLog...
none
New patch ggaren: review+

Aaron Golden
Reported 2008-11-07 17:00:25 PST
The for loop to iterate over the children vector in -[WebHistoryItem dictionaryRepresentation] starts out with i = children.size(), so if we ever hit that loop we're going to attempt an access past the end of the children vector. It should be i = children.size()-1.
Attachments
This patch prevents the bad access. (674 bytes, patch)
2008-11-07 17:05 PST, Aaron Golden 		no flags
DetailsFormatted DiffDiff
Same patch as before, but with a bit more whitespace and a ChangeLog... (1.12 KB, patch)
2008-11-07 17:11 PST, Aaron Golden 		no flags
DetailsFormatted DiffDiff
New patch (1.61 KB, patch)
2008-11-10 12:42 PST, Aaron Golden 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Aaron Golden
Comment 1 2008-11-07 17:05:48 PST
Created attachment 24979 [details] This patch prevents the bad access.
Aaron Golden
Comment 2 2008-11-07 17:11:58 PST
Created attachment 24980 [details] Same patch as before, but with a bit more whitespace and a ChangeLog...
Darin Adler
Comment 3 2008-11-09 11:50:56 PST
Comment on attachment 24980 [details] Same patch as before, but with a bit more whitespace and a ChangeLog... This change is clearly correct, but we normally require regression tests for bug fixes. Were you able to reproduce a problem? I'd love to see a test and not just the fix. r=me
Aaron Golden
Comment 4 2008-11-10 12:42:30 PST
Created attachment 25025 [details] New patch It turns out that -[WebHistoryItem initWithDictionaryRepresentation] has the same problem as -[WebHistoryItem dictionaryRepresentation] so fixing that as well and updating the ChangeLog.
David Kilzer (:ddkilzer)
Comment 5 2008-11-10 14:42:42 PST
Comment on attachment 24980 [details] Same patch as before, but with a bit more whitespace and a ChangeLog... Clearing review flag on unlanded patch.
Aaron Golden
Comment 6 2008-11-10 15:06:05 PST
(In reply to comment #3) > (From update of attachment 24980 [details] [review]) > This change is clearly correct, but we normally require regression tests for > bug fixes. Were you able to reproduce a problem? I'd love to see a test and not > just the fix. > > r=me > I think that the affected API is not currently used in the Safari browser, so I will not be able to provide a layout test. In theory I could write a new application designed to demonstrate the problem, but in this case I doubt that it's necessary.
Geoffrey Garen
Comment 7 2008-11-11 09:24:43 PST
Comment on attachment 25025 [details] New patch r=me
David Kilzer (:ddkilzer)
Comment 8 2008-11-11 14:14:59 PST
$ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/mac/ChangeLog M WebKit/mac/History/WebHistoryItem.mm Committed r38315
Note You need to log in before you can comment on or make changes to this bug.