Summary: | Reinforce Geolocation to prevent accidental leak of the user position | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||
Component: | WebCore Misc. | Assignee: | Benjamin Poulain <benjamin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, ddkilzer | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Benjamin Poulain
2012-03-27 16:42:31 PDT
Related to <rdar://problem/11106417> Created attachment 134171 [details]
Patch
Comment on attachment 134171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134171&action=review > Source/WebCore/Modules/geolocation/Geolocation.cpp:128 > + // If we are here and the Geolocation permission is not approved, something has > + // gone horribly wrong. > + // We bail out to avoid any privacy issue. > + ASSERT(m_geolocation->isAllowed()); > + if (!m_geolocation->isAllowed()) > + return; Should we CRASH() rather than return here? > > Source/WebCore/Modules/geolocation/Geolocation.cpp:128
> > + // If we are here and the Geolocation permission is not approved, something has
> > + // gone horribly wrong.
> > + // We bail out to avoid any privacy issue.
> > + ASSERT(m_geolocation->isAllowed());
> > + if (!m_geolocation->isAllowed())
> > + return;
>
> Should we CRASH() rather than return here?
I have considered that but that seemed a bit extreme.
Do you have any input on what is bad enough that we should take down the whole browser? It is difficult to know it an error comes from a security breach or a programming mistake.
> Do you have any input on what is bad enough that we should take down the whole browser? It is difficult to know it an error comes from a security breach or a programming mistake.
It's up to you. The nice thing about crashing in impossible situations is that we'll get crash reports and we can fix the issue. :)
Committed r112347: <http://trac.webkit.org/changeset/112347> |