180688 2017-12-11 17:47:04 -0800 Fix possible out-of-bounds read in protocolIsInHTTPFamily 2017-12-14 18:04:36 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 achristensen achristensen darin dbates webkit-bug-importer oldest_to_newest 1380493 0 achristensen 2017-12-11 17:47:04 -0800 Fix possible out-of-bounds read in protocolIsInHTTPFamily 1380496 1 329067 achristensen 2017-12-11 17:48:36 -0800 Created attachment 329067 Patch 1380505 2 329070 achristensen 2017-12-11 18:06:30 -0800 Created attachment 329070 Patch 1380851 3 329070 dbates 2017-12-12 14:25:17 -0800 Comment on attachment 329070 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329070&action=review > Source/WebCore/platform/URL.cpp:877 > + return url.length() > 4 We could cache url.length() in a local to avoid dereferencing it twice. Maybe the compiler is smart enough? For your consideration I suggest using >= 5 as it tends to be easier to reason about when verifying bounds. The compiler should be smart enough to optimize this to > 4 if such a micro-optimization is considered a win on a particular ISA. > Source/WebCore/platform/URL.cpp:882 > + && (url[4] == ':' || (isASCIIAlphaCaselessEqual(url[4], 's') && url.length() > 5 && url[5] == ':')); For similar reasons I suggest changing > 5 to >= 6. > Tools/TestWebKitAPI/Tests/WebCore/URL.cpp:218 > + EXPECT_FALSE(protocolIsInHTTPFamily("abc")); I suggest that we also check the empty string, a null string, and a string with a single character. 1380988 4 achristensen 2017-12-12 17:54:44 -0800 http://trac.webkit.org/r225829 1380989 5 webkit-bug-importer 2017-12-12 17:55:20 -0800 <rdar://problem/36010694> 1381002 6 329070 darin 2017-12-12 18:26:57 -0800 Comment on attachment 329070 Patch This bug is based on false promise. The String subscript operate return zero for characters past the end of the string, This change would be needed if we change the argument type to something like StringView or if we change the behavior of the string class subscript operator. But otherwise it is unnecessary to make a change. 1381504 7 darin 2017-12-13 21:24:21 -0800 (In reply to Darin Adler from comment #6) > false promise false premise(In reply to Darin Adler from comment #6) > subscript operate return zero subscript operator returns zero 1381963 8 achristensen 2017-12-14 18:04:36 -0800 Dan and I disagree about what to do here. This was safe before because of our implementation of WTF::String::operator[]. Dan thinks we should revert this change and encourage programmers to rely on String's bounds checks. I think that this was still a good change because it makes the code look safer and it probably doesn't change the optimized code generated. I don't think we should encourage dangerous-looking code in WebKit, and I think WTF::String::operator[] checks should be a fallback to make sure when we make a mistake it doesn't turn into a security issue. 329067 2017-12-11 17:48:36 -0800 2017-12-11 18:06:28 -0800 Patch bug-180688-20171211174835.patch text/plain 3126 achristensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIyNTc0MikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDE3LTEyLTExICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgRml4IHBvc3Np YmxlIG91dC1vZi1ib3VuZHMgcmVhZCBpbiBwcm90b2NvbElzSW5IVFRQRmFtaWx5CisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODA2ODgKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJdCB3b3VsZG4ndCByZWFk IHZlcnkgZmFyIG91dCBvZiBib3VuZHMsIGFuZCBpdCB3b3VsZCBqdXN0IGNoYW5nZSBhIGJvb2wg cmV0dXJuIHZhbHVlLAorICAgICAgICBidXQgaXQncyBzdGlsbCBvdXQgb2YgYm91bmRzLiAgQ292 ZXJlZCBieSBhbiBBUEkgdGVzdCB0aGF0IEFTQU4gd291bGRuJ3QgbGlrZS4KKworICAgICAgICAq IHBsYXRmb3JtL1VSTC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpwcm90b2NvbElzSW5IVFRQRmFt aWx5KToKKyAgICAgICAgQ2hlY2sgYm91bmRzIGJlZm9yZSByZWFkaW5nIGEgc3RyaW5nLgorCiAy MDE3LTEyLTExICBNYW51ZWwgUmVnbyBDYXNhc25vdmFzICA8cmVnb0BpZ2FsaWEuY29tPgogCiAg ICAgICAgIFJFR1JFU1NJT04ocjIyMTkzMSk6IFJvdyBzdHJldGNoIGRvZXNuJ3Qgd29yayBmb3Ig Z3JpZCBjb250YWluZXIgd2l0aCBtaW4taGVpZ2h0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9VUkwuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1VSTC5j cHAJKHJldmlzaW9uIDIyNTc0MikKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1VSTC5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTg3NCwxMSArODc0LDEyIEBAIGJvb2wgcHJvdG9jb2xJc0phdmFT Y3JpcHQoU3RyaW5nVmlldyB1cmwKIGJvb2wgcHJvdG9jb2xJc0luSFRUUEZhbWlseShjb25zdCBT dHJpbmcmIHVybCkKIHsKICAgICAvLyBEbyB0aGUgY29tcGFyaXNvbiB3aXRob3V0IG1ha2luZyBh IG5ldyBzdHJpbmcgb2JqZWN0LgotICAgIHJldHVybiBpc0FTQ0lJQWxwaGFDYXNlbGVzc0VxdWFs KHVybFswXSwgJ2gnKQorICAgIHJldHVybiB1cmwubGVuZ3RoKCkgPiA0CisgICAgICAgICYmIGlz QVNDSUlBbHBoYUNhc2VsZXNzRXF1YWwodXJsWzBdLCAnaCcpCiAgICAgICAgICYmIGlzQVNDSUlB bHBoYUNhc2VsZXNzRXF1YWwodXJsWzFdLCAndCcpCiAgICAgICAgICYmIGlzQVNDSUlBbHBoYUNh c2VsZXNzRXF1YWwodXJsWzJdLCAndCcpCiAgICAgICAgICYmIGlzQVNDSUlBbHBoYUNhc2VsZXNz RXF1YWwodXJsWzNdLCAncCcpCi0gICAgICAgICYmICh1cmxbNF0gPT0gJzonIHx8IChpc0FTQ0lJ QWxwaGFDYXNlbGVzc0VxdWFsKHVybFs0XSwgJ3MnKSAmJiB1cmxbNV0gPT0gJzonKSk7CisgICAg ICAgICYmICh1cmxbNF0gPT0gJzonIHx8IChpc0FTQ0lJQWxwaGFDYXNlbGVzc0VxdWFsKHVybFs0 XSwgJ3MnKSAmJiB1cmwubGVuZ3RoID4gNSAmJiB1cmxbNV0gPT0gJzonKSk7CiB9CiAKIGNvbnN0 IFVSTCYgYmxhbmtVUkwoKQpJbmRleDogVG9vbHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRv b2xzL0NoYW5nZUxvZwkocmV2aXNpb24gMjI1NzY2KQorKysgVG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTctMTItMTEgIEFsZXggQ2hyaXN0ZW5zZW4g IDxhY2hyaXN0ZW5zZW5Ad2Via2l0Lm9yZz4KKworICAgICAgICBGaXggcG9zc2libGUgb3V0LW9m LWJvdW5kcyByZWFkIGluIHByb3RvY29sSXNJbkhUVFBGYW1pbHkKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4MDY4OAorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJD b3JlL1VSTC5jcHA6CisgICAgICAgIChUZXN0V2ViS2l0QVBJOjpURVNUX0YpOgorCiAyMDE3LTEy LTExICBEYXZpZCBRdWVzYWRhICA8ZGF2aWRfcXVlc2FkYUBhcHBsZS5jb20+CiAKICAgICAgICAg VHVybiBvbiBFTkFCTEVfQVBQTElDQVRJT05fTUFOSUZFU1QKSW5kZXg6IFRvb2xzL1Rlc3RXZWJL aXRBUEkvVGVzdHMvV2ViQ29yZS9VUkwuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1Rlc3RXZWJL aXRBUEkvVGVzdHMvV2ViQ29yZS9VUkwuY3BwCShyZXZpc2lvbiAyMjU3NDIpCisrKyBUb29scy9U ZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYkNvcmUvVVJMLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjEz LDQgKzIxMywxNCBAQCBURVNUX0YoVVJMVGVzdCwgVVJMUmVtb3ZlUXVlcnlBbmRGcmFnbWVuCiAg ICAgRVhQRUNUX0VRKHVybC5zdHJpbmcoKSwgdXJsNS5zdHJpbmcoKSk7CiB9CiAKK1RFU1RfRihV UkxUZXN0LCBQcm90b2NvbElzSW5IVFRQRmFtaWx5KQoreworICAgIEVYUEVDVF9GQUxTRShwcm90 b2NvbElzSW5IVFRQRmFtaWx5KCJhYmMiKSk7CisgICAgRVhQRUNUX1RSVUUocHJvdG9jb2xJc0lu SFRUUEZhbWlseSgiaHR0cDoiKSk7CisgICAgRVhQRUNUX0ZBTFNFKHByb3RvY29sSXNJbkhUVFBG YW1pbHkoImh0dHAiKSk7CisgICAgRVhQRUNUX1RSVUUocHJvdG9jb2xJc0luSFRUUEZhbWlseSgi aHR0cHM6IikpOworICAgIEVYUEVDVF9GQUxTRShwcm90b2NvbElzSW5IVFRQRmFtaWx5KCJodHRw cyIpKTsKKyAgICBFWFBFQ1RfVFJVRShwcm90b2NvbElzSW5IVFRQRmFtaWx5KCJodHRwczovLyFA IyQlXiYqKCkiKSk7Cit9CisKIH0gLy8gbmFtZXNwYWNlIFRlc3RXZWJLaXRBUEkK 329070 2017-12-11 18:06:30 -0800 2017-12-12 14:25:17 -0800 Patch bug-180688-20171211180629.patch text/plain 3128 achristensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIyNTc0MikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDE3LTEyLTExICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgRml4IHBvc3Np YmxlIG91dC1vZi1ib3VuZHMgcmVhZCBpbiBwcm90b2NvbElzSW5IVFRQRmFtaWx5CisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODA2ODgKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJdCB3b3VsZG4ndCByZWFk IHZlcnkgZmFyIG91dCBvZiBib3VuZHMsIGFuZCBpdCB3b3VsZCBqdXN0IGNoYW5nZSBhIGJvb2wg cmV0dXJuIHZhbHVlLAorICAgICAgICBidXQgaXQncyBzdGlsbCBvdXQgb2YgYm91bmRzLiAgQ292 ZXJlZCBieSBhbiBBUEkgdGVzdCB0aGF0IEFTQU4gd291bGRuJ3QgbGlrZS4KKworICAgICAgICAq IHBsYXRmb3JtL1VSTC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpwcm90b2NvbElzSW5IVFRQRmFt aWx5KToKKyAgICAgICAgQ2hlY2sgYm91bmRzIGJlZm9yZSByZWFkaW5nIGEgc3RyaW5nLgorCiAy MDE3LTEyLTExICBNYW51ZWwgUmVnbyBDYXNhc25vdmFzICA8cmVnb0BpZ2FsaWEuY29tPgogCiAg ICAgICAgIFJFR1JFU1NJT04ocjIyMTkzMSk6IFJvdyBzdHJldGNoIGRvZXNuJ3Qgd29yayBmb3Ig Z3JpZCBjb250YWluZXIgd2l0aCBtaW4taGVpZ2h0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9VUkwuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1VSTC5j cHAJKHJldmlzaW9uIDIyNTc0MikKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1VSTC5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTg3NCwxMSArODc0LDEyIEBAIGJvb2wgcHJvdG9jb2xJc0phdmFT Y3JpcHQoU3RyaW5nVmlldyB1cmwKIGJvb2wgcHJvdG9jb2xJc0luSFRUUEZhbWlseShjb25zdCBT dHJpbmcmIHVybCkKIHsKICAgICAvLyBEbyB0aGUgY29tcGFyaXNvbiB3aXRob3V0IG1ha2luZyBh IG5ldyBzdHJpbmcgb2JqZWN0LgotICAgIHJldHVybiBpc0FTQ0lJQWxwaGFDYXNlbGVzc0VxdWFs KHVybFswXSwgJ2gnKQorICAgIHJldHVybiB1cmwubGVuZ3RoKCkgPiA0CisgICAgICAgICYmIGlz QVNDSUlBbHBoYUNhc2VsZXNzRXF1YWwodXJsWzBdLCAnaCcpCiAgICAgICAgICYmIGlzQVNDSUlB bHBoYUNhc2VsZXNzRXF1YWwodXJsWzFdLCAndCcpCiAgICAgICAgICYmIGlzQVNDSUlBbHBoYUNh c2VsZXNzRXF1YWwodXJsWzJdLCAndCcpCiAgICAgICAgICYmIGlzQVNDSUlBbHBoYUNhc2VsZXNz RXF1YWwodXJsWzNdLCAncCcpCi0gICAgICAgICYmICh1cmxbNF0gPT0gJzonIHx8IChpc0FTQ0lJ QWxwaGFDYXNlbGVzc0VxdWFsKHVybFs0XSwgJ3MnKSAmJiB1cmxbNV0gPT0gJzonKSk7CisgICAg ICAgICYmICh1cmxbNF0gPT0gJzonIHx8IChpc0FTQ0lJQWxwaGFDYXNlbGVzc0VxdWFsKHVybFs0 XSwgJ3MnKSAmJiB1cmwubGVuZ3RoKCkgPiA1ICYmIHVybFs1XSA9PSAnOicpKTsKIH0KIAogY29u c3QgVVJMJiBibGFua1VSTCgpCkluZGV4OiBUb29scy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g VG9vbHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMjU3NjYpCisrKyBUb29scy9DaGFuZ2VMb2cJKHdv cmtpbmcgY29weSkKQEAgLTEsMyArMSwxMyBAQAorMjAxNy0xMi0xMSAgQWxleCBDaHJpc3RlbnNl biAgPGFjaHJpc3RlbnNlbkB3ZWJraXQub3JnPgorCisgICAgICAgIEZpeCBwb3NzaWJsZSBvdXQt b2YtYm91bmRzIHJlYWQgaW4gcHJvdG9jb2xJc0luSFRUUEZhbWlseQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgwNjg4CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBUZXN0V2ViS2l0QVBJL1Rlc3RzL1dl YkNvcmUvVVJMLmNwcDoKKyAgICAgICAgKFRlc3RXZWJLaXRBUEk6OlRFU1RfRik6CisKIDIwMTct MTItMTEgIERhdmlkIFF1ZXNhZGEgIDxkYXZpZF9xdWVzYWRhQGFwcGxlLmNvbT4KIAogICAgICAg ICBUdXJuIG9uIEVOQUJMRV9BUFBMSUNBVElPTl9NQU5JRkVTVApJbmRleDogVG9vbHMvVGVzdFdl YktpdEFQSS9UZXN0cy9XZWJDb3JlL1VSTC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gVG9vbHMvVGVzdFdl YktpdEFQSS9UZXN0cy9XZWJDb3JlL1VSTC5jcHAJKHJldmlzaW9uIDIyNTc0MikKKysrIFRvb2xz L1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViQ29yZS9VUkwuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0y MTMsNCArMjEzLDE0IEBAIFRFU1RfRihVUkxUZXN0LCBVUkxSZW1vdmVRdWVyeUFuZEZyYWdtZW4K ICAgICBFWFBFQ1RfRVEodXJsLnN0cmluZygpLCB1cmw1LnN0cmluZygpKTsKIH0KIAorVEVTVF9G KFVSTFRlc3QsIFByb3RvY29sSXNJbkhUVFBGYW1pbHkpCit7CisgICAgRVhQRUNUX0ZBTFNFKHBy b3RvY29sSXNJbkhUVFBGYW1pbHkoImFiYyIpKTsKKyAgICBFWFBFQ1RfVFJVRShwcm90b2NvbElz SW5IVFRQRmFtaWx5KCJodHRwOiIpKTsKKyAgICBFWFBFQ1RfRkFMU0UocHJvdG9jb2xJc0luSFRU UEZhbWlseSgiaHR0cCIpKTsKKyAgICBFWFBFQ1RfVFJVRShwcm90b2NvbElzSW5IVFRQRmFtaWx5 KCJodHRwczoiKSk7CisgICAgRVhQRUNUX0ZBTFNFKHByb3RvY29sSXNJbkhUVFBGYW1pbHkoImh0 dHBzIikpOworICAgIEVYUEVDVF9UUlVFKHByb3RvY29sSXNJbkhUVFBGYW1pbHkoImh0dHBzOi8v IUAjJCVeJiooKSIpKTsKK30KKwogfSAvLyBuYW1lc3BhY2UgVGVzdFdlYktpdEFQSQo=