Summary: | [Chromium] Confirm index is valid in BackForwardListClientImpl::itemAtIndex() before returning non-null | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nate Chapin <japhet> | ||||||
Component: | WebCore Misc. | Assignee: | Nate Chapin <japhet> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | scarybeasts | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Nate Chapin
2010-02-08 13:17:54 PST
Created attachment 48364 [details]
patch
Comment on attachment 48364 [details] patch Please add a link to this bug in the ChangeLog entry. > Index: WebKit/chromium/src/BackForwardListClientImpl.cpp > =================================================================== > --- WebKit/chromium/src/BackForwardListClientImpl.cpp (revision 54505) > +++ WebKit/chromium/src/BackForwardListClientImpl.cpp (working copy) > @@ -90,7 +90,7 @@ > > HistoryItem* BackForwardListClientImpl::itemAtIndex(int index) > { > - if (!m_webView->client()) > + if (!m_webView->client() || index > forwardListCount() || index < (backListCount() * -1)) It might read slightly better as: if (!m_webView->client() || index > forwardListCount() || -index > backListCount()) Either way, R=me Please file a bug about adding a layout test per our discussion. For those reading at home, Nate's initial attempts at creating a test didn't pan out, and this bug is a release blocker for Chrome, so we are splitting the task into two bugs. He'll work on completing the layout test next. Comment on attachment 48364 [details] patch http://trac.webkit.org/changeset/54511 If it's alright, I'll leave this bug open for the layout test that I will be submitting ASAP. Comment on attachment 48364 [details] patch Cleared Darin Fisher's review+ from obsolete attachment 48364 [details] so that this bug does not appear in http://webkit.org/pending-commit. Created attachment 48375 [details]
layout test
Sorry for the delay in getting this test out.
Do we know what revision broke this? Comment on attachment 48375 [details]
layout test
I support all passing tests.
|