Geolocation clearWatch() needs to protect against invalid IDs

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Steve Block]

Geolocation uses HashMap to store watch requests, keyed off the watch ID. Key values of 0 or -1 must not be used with HashMap. Geolocation does not use these values internally, but we need to protect against them being passed to clearWatch() from JavaScript.

This was first reported in https://bugs.webkit.org/show_bug.cgi?id=39879#c60

[Steve Block]

Created attachment 64570 [details]
Patch

[Jeremy Orlow]

Comment on attachment 64570 [details]
Patch

r=me

Why would this crash though?  Maybe the problem should be fixed within HashMap (or whatever's causing it)?

[Darin Adler]

(In reply to comment #2)
> Maybe the problem should be fixed within HashMap (or whatever's causing it)?

Changing HashMap itself directly is probably out of the question. HashMap achieves its speed in part by reserving values. We could use a hash table with a different design, but we’ve repeatedly reaffirmed our desire to use this.

Putting the fix closer to the use of the HashMap object would make sense for some call sites like these. We could add some new functions to HashMap called safeFind and safeGet that would work more slowly and check for the empty and deleted values, for uses in sites like these where we need the reliable but slower behavior. That might be a cleaner fix than the higher level validity checks.

[WebKit Commit Bot]

Comment on attachment 64570 [details]
Patch

Clearing flags on attachment: 64570

Committed r65570: <http://trac.webkit.org/changeset/65570>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.