Bug 82396 – Reinforce Geolocation to prevent accidental leak of the user position

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Benjamin Poulain]

Some simple changes can be done to enhance Geolocation in case of programming mistake.

[Benjamin Poulain]

Related to <rdar://problem/11106417>

[Benjamin Poulain]

Created attachment 134171 [details]
Patch

[Adam Barth]

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?

[Benjamin Poulain]

> > 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.

[Adam Barth]

> 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.  :)

[Benjamin Poulain]

Committed r112347: <http://trac.webkit.org/changeset/112347>