Bug 214669 – Web Inspector: developerExtrasEnabled should be respected when opening local Web Inspector (part 2)

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

.

[Blaze Burg]

Created attachment 405004 [details]
Patch

[Blaze Burg]

<rdar://65885126>

[Joseph Pecoraro]

Comment on attachment 405004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405004&action=review

> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:671
> +    if (!m_inspectedPage->preferences().developerExtrasEnabled())
> +        return;

How would it be possible to get here?

[Blaze Burg]

Comment on attachment 405004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405004&action=review

>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:671
>> +        return;
> 
> How would it be possible to get here?

A compromised WebContent process may try to trick UIProcess into using Inspector functionality even if it's disabled. We don't want that to happen.

[Joseph Pecoraro]

Comment on attachment 405004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405004&action=review

>>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:671
>>> +        return;
>> 
>> How would it be possible to get here?
> 
> A compromised WebContent process may try to trick UIProcess into using Inspector functionality even if it's disabled. We don't want that to happen.

Sounds good. Is this the only command then? It seems `WebInspectorProxy::append` could be concerning as well.

[Devin Rousso]

Comment on attachment 405004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405004&action=review

>>>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:671
>>>> +        return;
>>> 
>>> How would it be possible to get here?
>> 
>> A compromised WebContent process may try to trick UIProcess into using Inspector functionality even if it's disabled. We don't want that to happen.
> 
> Sounds good. Is this the only command then? It seems `WebInspectorProxy::append` could be concerning as well.

Based on this logic there should probably be a check for `WebInspectorProxy::append` too.

[Devin Rousso]

ah lol @Joe beat me to it :P

r=me as well :)

[Blaze Burg]

Comment on attachment 405004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405004&action=review

>>>>> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:671
>>>>> +        return;
>>>> 
>>>> How would it be possible to get here?
>>> 
>>> A compromised WebContent process may try to trick UIProcess into using Inspector functionality even if it's disabled. We don't want that to happen.
>> 
>> Sounds good. Is this the only command then? It seems `WebInspectorProxy::append` could be concerning as well.
> 
> Based on this logic there should probably be a check for `WebInspectorProxy::append` too.

I'll address ::append as well.

[Blaze Burg]

Created attachment 405082 [details]
Patch

[EWS]

Committed r264803: <https://trac.webkit.org/changeset/264803>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405082 [details].