43203 2010-07-29 12:36:24 -0700 WebBackForwardList::back/ForwardListWithLimit() crashes if passed a limit larger than max int 2010-07-29 14:36:06 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 sullivan sullivan aroben oldest_to_newest 257811 0 sullivan 2010-07-29 12:36:24 -0700 WebBackForwardList::backListWithLimit() and forwardListWithLimit() crash if the limit parameter, an unsigned value, is larger than numeric_limits<int>::max(). The crash occurs here, with i == 0 but an empty m_entries: WebBackForwardListItem* item = m_entries[i].get(); The crash occurs due to this incorrect logic: unsigned size = std::min(backListCount(), static_cast<unsigned>(limit)); if (!size) return ImmutableArray::create(); This is attempting to return early for the empty case, but casting the unsigned limit to an int can make it negative, and thus size is negative, and thus the test for !size fails. I've got a fix that I'll send out after lunch, if nobody beats me to it. 257863 1 62989 sullivan 2010-07-29 13:59:53 -0700 Created attachment 62989 Patch to cast to unsigned rather than int, to avoid wrapping 257869 2 62989 aroben 2010-07-29 14:06:05 -0700 Comment on attachment 62989 Patch to cast to unsigned rather than int, to avoid wrapping > - unsigned size = std::min(backListCount(), static_cast<int>(limit)); > + unsigned size = std::min(static_cast<unsigned>(backListCount()), limit); Why does backListCount return an int? Seems like it should return unsigned. 257882 3 sullivan 2010-07-29 14:24:41 -0700 I agree that backForwardCount() should not return an int. Probably all of these functions should deal with size_t's. But I didn't want to get into that territory for this fix. Checked in as http://trac.webkit.org/changeset/64306 257894 4 sullivan 2010-07-29 14:36:06 -0700 I filed a bug about the inconsistent use of types in this area: <https://bugs.webkit.org/show_bug.cgi?id=43214> 62989 2010-07-29 13:59:53 -0700 2010-07-29 14:06:05 -0700 Patch to cast to unsigned rather than int, to avoid wrapping backListCastingPatch.txt text/plain 1812 sullivan SW5kZXg6IFdlYktpdDIvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYktpdDIvQ2hhbmdlTG9n CShyZXZpc2lvbiA2NDMwNCkKKysrIFdlYktpdDIvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDctMjkgIEpvaG4gU3VsbGl2YW4gIDxzdWxsaXZhbkBhcHBs ZS5jb20+CisKKyAgICAgICAgPGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD00MzIwMz4KKyAgICAgICAgV2ViQmFja0ZvcndhcmRMaXN0OjpiYWNrL0ZvcndhcmRMaXN0V2l0 aExpbWl0KCkgY3Jhc2hlcyBpZiBwYXNzZWQgYSBsaW1pdCBsYXJnZXIgdGhhbiBtYXggaW50CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVSVByb2Nl c3MvV2ViQmFja0ZvcndhcmRMaXN0LmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViQmFja0Zvcndh cmRMaXN0OjpiYWNrTGlzdEFzSW1tdXRhYmxlQXJyYXlXaXRoTGltaXQpOgorICAgICAgICBGaXhl ZCBjYXN0aW5nIHNvIHRoYXQgYSBsYXJnZSB1bnNpZ25lZCB3b24ndCBiZWNvbWUgYSBuZWdhdGl2 ZSBpbnQuCisgICAgICAgIChXZWJLaXQ6OldlYkJhY2tGb3J3YXJkTGlzdDo6Zm9yd2FyZExpc3RB c0ltbXV0YWJsZUFycmF5V2l0aExpbWl0KToKKyAgICAgICAgRGl0dG8uCisKIDIwMTAtMDctMjkg IEFuZGVycyBDYXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdl ZCBieSBTYW0gV2VpbmlnLgpJbmRleDogV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQmFja0ZvcndhcmRM aXN0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXQyL1VJUHJvY2Vzcy9XZWJCYWNrRm9yd2FyZExp c3QuY3BwCShyZXZpc2lvbiA2NDI4MCkKKysrIFdlYktpdDIvVUlQcm9jZXNzL1dlYkJhY2tGb3J3 YXJkTGlzdC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE4MCw3ICsxODAsNyBAQCBzdGF0aWMgdm9p ZCB3ZWJCYWNrRm9yd2FyZExpc3RJdGVtRGVyZWYoCiAKIFBhc3NSZWZQdHI8SW1tdXRhYmxlQXJy YXk+IFdlYkJhY2tGb3J3YXJkTGlzdDo6YmFja0xpc3RBc0ltbXV0YWJsZUFycmF5V2l0aExpbWl0 KHVuc2lnbmVkIGxpbWl0KQogewotICAgIHVuc2lnbmVkIHNpemUgPSBzdGQ6Om1pbihiYWNrTGlz dENvdW50KCksIHN0YXRpY19jYXN0PGludD4obGltaXQpKTsKKyAgICB1bnNpZ25lZCBzaXplID0g c3RkOjptaW4oc3RhdGljX2Nhc3Q8dW5zaWduZWQ+KGJhY2tMaXN0Q291bnQoKSksIGxpbWl0KTsK ICAgICBpZiAoIXNpemUpCiAgICAgICAgIHJldHVybiBJbW11dGFibGVBcnJheTo6Y3JlYXRlKCk7 CiAKQEAgLTIwMCw3ICsyMDAsNyBAQCBQYXNzUmVmUHRyPEltbXV0YWJsZUFycmF5PiBXZWJCYWNr Rm9yd2FyCiAKIFBhc3NSZWZQdHI8SW1tdXRhYmxlQXJyYXk+IFdlYkJhY2tGb3J3YXJkTGlzdDo6 Zm9yd2FyZExpc3RBc0ltbXV0YWJsZUFycmF5V2l0aExpbWl0KHVuc2lnbmVkIGxpbWl0KQogewot ICAgIHVuc2lnbmVkIHNpemUgPSBzdGQ6Om1pbihmb3J3YXJkTGlzdENvdW50KCksIHN0YXRpY19j YXN0PGludD4obGltaXQpKTsKKyAgICB1bnNpZ25lZCBzaXplID0gc3RkOjptaW4oc3RhdGljX2Nh c3Q8dW5zaWduZWQ+KGZvcndhcmRMaXN0Q291bnQoKSksIGxpbWl0KTsKICAgICBpZiAoIXNpemUp CiAgICAgICAgIHJldHVybiBJbW11dGFibGVBcnJheTo6Y3JlYXRlKCk7CiAK