Bug 43854

Summary: Web Inspector: remove openInInspectorWindow and evaluateAndStringify from InjectedScript.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed change.
yurys: review-
[PATCH] Proposed change none

Description Pavel Feldman 2010-08-11 08:41:31 PDT 
Patch to follow.
Comment 1 Pavel Feldman 2010-08-11 08:42:42 PDT 
Created attachment 64118 [details]
[PATCH] Proposed change.
Comment 2 WebKit Review Bot 2010-08-11 08:47:14 PDT 
Attachment 64118 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/inspector/InspectorController.cpp:50:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/InspectorController.cpp:1930:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Yury Semikhatsky 2010-08-11 23:22:50 PDT 
Comment on attachment 64118 [details]
[PATCH] Proposed change.

WebCore/inspector/InspectorController.cpp:1884
 +      newFrame->loader()->setOpener(mainFrame);
We may want to break this link for security reasons(to avoid possibility for the inspected page to open an arbitrary URL by means of the web inspector and have an access to it).

WebCore/inspector/InspectorController.cpp:1925
 +  InjectedScript InspectorController::injectedScriptForId(long id)
It's not used anywhere, please remove.

WebCore/inspector/front-end/InjectedScriptAccess.js: 
 +  InjectedScriptAccess._installHandler("evaluateAndStringify");
Please fix the callers.
Comment 4 Pavel Feldman 2010-08-13 04:55:56 PDT 
(In reply to comment #3)
> (From update of attachment 64118 [details])
> WebCore/inspector/InspectorController.cpp:1884
>  +      newFrame->loader()->setOpener(mainFrame);
> We may want to break this link for security reasons(to avoid possibility for the inspected page to open an arbitrary URL by means of the web inspector and have an access to it).
> 

It is not worse than it used to be + I don't think it is exploitable.

> WebCore/inspector/InspectorController.cpp:1925
>  +  InjectedScript InspectorController::injectedScriptForId(long id)
> It's not used anywhere, please remove.
> 

Done.

> WebCore/inspector/front-end/InjectedScriptAccess.js: 
>  +  InjectedScriptAccess._installHandler("evaluateAndStringify");
> Please fix the callers.


Done.
Comment 5 Pavel Feldman 2010-08-13 04:56:43 PDT 
Created attachment 64320 [details]
[PATCH] Proposed change
Comment 6 Pavel Feldman 2010-08-13 05:08:46 PDT 
Comment on attachment 64320 [details]
[PATCH] Proposed change

Clearing flags on attachment: 64320

Committed r65314: <http://trac.webkit.org/changeset/65314>
Comment 7 Pavel Feldman 2010-08-13 05:08:56 PDT 
All reviewed patches have been landed.  Closing bug.