30122 2009-10-06 05:52:24 -0700 Geolocation does not protect against wrap-around of request IDs 2009-10-28 12:23:51 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 29178 1 steveblock steveblock bolsinga commit-queue darin eric steveblock oldest_to_newest 152689 0 steveblock 2009-10-06 05:52:24 -0700 Currently there is no guard to prevent wrap-around of the IDs used for Geolocation requests. This means that when the ID overflows, a previously existing request will be canceled and its ID reused. The orignal ID will then reference the wrong request. 152788 1 steveblock 2009-10-06 10:49:55 -0700 I looked at how window.setTimeout() handles overflow of its timer IDs. It looks like it simply allows the overflow to happen (while ensuring the ID remains positive), thus overwriting the previous timer. See http://trac.webkit.org/browser/trunk/WebCore/page/DOMTimer.cpp. I suggest we do the same for Geolocation requests - allow the overflow, making sure the ID remains positive or negative as appropriate. 158636 2 42043 steveblock 2009-10-28 10:46:24 -0700 Created attachment 42043 Patch 1 for Bug 30122 It's not practical to add a test for this, as triggering the overflow would require so many watches to be started that it would be prohibitively slow. 158638 3 42043 darin 2009-10-28 10:49:29 -0700 Comment on attachment 42043 Patch 1 for Bug 30122 r=me If you want to be pedantic, overflow has defined behavior for unsigned but not for int. So it's best to code this sort of thing so it detects overflow before it happens or use unsigned. But that's not a realistic concern. 158657 4 42043 commit-queue 2009-10-28 12:23:46 -0700 Comment on attachment 42043 Patch 1 for Bug 30122 Clearing flags on attachment: 42043 Committed r50229: <http://trac.webkit.org/changeset/50229> 158658 5 commit-queue 2009-10-28 12:23:51 -0700 All reviewed patches have been landed. Closing bug. 42043 2009-10-28 10:46:24 -0700 2009-10-28 12:23:46 -0700 Patch 1 for Bug 30122 handleWatchIdOverflow.txt text/plain 1290 steveblock SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MDIyMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDktMTAtMjggIFN0ZXZlIEJsb2NrICA8c3RldmVibG9ja0Bnb29n bGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE1ha2VzIHN1cmUgdGhhdCBHZW9sb2NhdGlvbiB3YXRjaCBJRHMgcmVtYWluIHBvc2l0aXZlIG9u IG92ZXJmbG93LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MzAxMjIKKworICAgICAgICBObyBuZXcgdGVzdHMgcG9zc2libGUuCisKKyAgICAgICAgKiBw YWdlL0dlb2xvY2F0aW9uLmNwcDogTW9kaWZpZWQuCisgICAgICAgIChXZWJDb3JlOjpHZW9sb2Nh dGlvbjo6d2F0Y2hQb3NpdGlvbik6IE1vZGlmaWVkLiBSZXNldCB0aGUgd2F0Y2ggSUQgdG8gMSBv biBvdmVyZmxvdy4KKwogMjAwOS0xMC0yOCAgUGF2ZWwgRmVsZG1hbiAgPHBmZWxkbWFuQGNocm9t aXVtLm9yZz4KIAogICAgICAgICBOb3QgcmV2aWV3ZWQ6IGZvbGxvdyB1cCBmaXggdG8gSW5zcGVj dG9yQ29udHJvbGxlclN0dWIuCkluZGV4OiBXZWJDb3JlL3BhZ2UvR2VvbG9jYXRpb24uY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYkNvcmUvcGFnZS9HZW9sb2NhdGlvbi5jcHAJKHJldmlzaW9uIDUwMjEz KQorKysgV2ViQ29yZS9wYWdlL0dlb2xvY2F0aW9uLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTY0 LDYgKzE2NCw5IEBAIGludCBHZW9sb2NhdGlvbjo6d2F0Y2hQb3NpdGlvbihQYXNzUmVmUHQKICAg ICAgICAgcmV0dXJuIDA7CiAKICAgICBzdGF0aWMgaW50IG5leHRBdmFpbGFibGVXYXRjaElkID0g MTsKKyAgICAvLyBJbiBjYXNlIG9mIG92ZXJmbG93LCBtYWtlIHN1cmUgdGhlIElEIHJlbWFpbnMg cG9zaXRpdmUsIGJ1dCByZXVzZSB0aGUgSUQgdmFsdWVzLgorICAgIGlmIChuZXh0QXZhaWxhYmxl V2F0Y2hJZCA8IDEpCisgICAgICAgIG5leHRBdmFpbGFibGVXYXRjaElkID0gMTsKICAgICBtX3dh dGNoZXJzLnNldChuZXh0QXZhaWxhYmxlV2F0Y2hJZCwgbm90aWZpZXIucmVsZWFzZSgpKTsKICAg ICByZXR1cm4gbmV4dEF2YWlsYWJsZVdhdGNoSWQrKzsKIH0K