Bug 44096 - Geolocation clearWatch() needs to protect against invalid IDs
Summary: Geolocation clearWatch() needs to protect against invalid IDs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-17 03:05 PDT by Steve Block
Modified: 2010-08-18 08:41 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.58 KB, patch)
2010-08-17 03:19 PDT, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2010-08-17 03:05:35 PDT
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
Comment 1 Steve Block 2010-08-17 03:19:44 PDT
Created attachment 64570 [details]
Patch
Comment 2 Jeremy Orlow 2010-08-17 13:36:12 PDT
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)?
Comment 3 Darin Adler 2010-08-17 14:09:58 PDT
(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.
Comment 4 WebKit Commit Bot 2010-08-17 16:06:07 PDT
Comment on attachment 64570 [details]
Patch

Clearing flags on attachment: 64570

Committed r65570: <http://trac.webkit.org/changeset/65570>
Comment 5 WebKit Commit Bot 2010-08-17 16:06:12 PDT
All reviewed patches have been landed.  Closing bug.