Bug 214669 - Web Inspector: developerExtrasEnabled should be respected when opening local Web Inspector (part 2)
Summary: Web Inspector: developerExtrasEnabled should be respected when opening local ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-22 18:07 PDT by BJ Burg
Modified: 2020-07-23 15:48 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.49 KB, patch)
2020-07-22 18:14 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch (1.90 KB, patch)
2020-07-23 15:15 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2020-07-22 18:07:04 PDT
.
Comment 1 BJ Burg 2020-07-22 18:14:24 PDT
Created attachment 405004 [details]
Patch
Comment 2 BJ Burg 2020-07-22 18:15:13 PDT
<rdar://65885126>
Comment 3 Joseph Pecoraro 2020-07-22 18:43:14 PDT
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?
Comment 4 BJ Burg 2020-07-23 13:43:11 PDT
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.
Comment 5 Joseph Pecoraro 2020-07-23 13:45:51 PDT
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.
Comment 6 Devin Rousso 2020-07-23 13:46:39 PDT
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.
Comment 7 Devin Rousso 2020-07-23 13:47:11 PDT
ah lol @Joe beat me to it :P

r=me as well :)
Comment 8 BJ Burg 2020-07-23 15:07:14 PDT
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.
Comment 9 BJ Burg 2020-07-23 15:15:47 PDT
Created attachment 405082 [details]
Patch
Comment 10 EWS 2020-07-23 15:48:33 PDT
Committed r264803: <https://trac.webkit.org/changeset/264803>

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