Bug 146040

Summary: Web Inspector: Inspector Scripts evaluated in the page should not be searchable
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, burg, commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
joepeck: review-, joepeck: commit-queue-
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2015-06-16 18:16:24 PDT
* SUMMARY
Inspector Scripts evaluated in the page should not be searchable.

* STEPS TO REPRODUCE
1. Inspect about:blank
2. Type "[1,2].con" in the console, to show autocompletion "concat"
3. Search "1,2" or "getComple"
  => results in Inspector internal script evaluated on the page to get completion for [1,2]

* NOTES
- The frontend ignores all evaluations containing a sourceURL starting with "__WebInspector", so we should append something like that for EVERY eval the frontend runs on the inspected page.
Comment 1 Radar WebKit Bug Importer 2015-06-16 18:16:55 PDT
<rdar://problem/21413971>
Comment 2 Joseph Pecoraro 2015-06-16 18:24:14 PDT
Also affects DebuggerPopovers, and other one-off evaluations like "document".

Basically all of these places can evaluate arbitrary code on the frontend:

  - RuntimeAgent.evaluate
  - DebuggerAgent.evaluateOnCallFrame
  - RuntimeAgent.callFunctionOn

Both of these are wrapped by managers / models, others are called directly in code. I will just look at updating the direct call sites of each to ensure I cover each case.
Comment 3 Joseph Pecoraro 2015-06-16 18:28:51 PDT
Created attachment 254991 [details]
[PATCH] Proposed Fix

cq- for the moment because at least one part of this has yet to be tested manually by me.
Comment 4 Joseph Pecoraro 2015-06-16 18:29:54 PDT
Comment on attachment 254991 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:159
> +            else if (result.type === "string" || result.type === "number" || result.type === "boolean") {
> +                var completionScript = "(" + getCompletions + ")(\"" + result.type + "\")";
> +                WebInspector.runtimeManager.evaluateInInspectedWindow(completionScript, "completion", false, true, true, false, false, receivedPropertyNamesFromEvaluate.bind(this));
> +            } else

Err, this change can be reverted now that I have evaluateInInspectedWindow add the sourceURL. Let me update the patch.
Comment 5 Joseph Pecoraro 2015-06-16 18:31:05 PDT
Created attachment 254992 [details]
[PATCH] Proposed Fix
Comment 6 WebKit Commit Bot 2015-06-16 22:12:30 PDT
Comment on attachment 254992 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 254992

Committed r185638: <http://trac.webkit.org/changeset/185638>
Comment 7 WebKit Commit Bot 2015-06-16 22:12:36 PDT
All reviewed patches have been landed.  Closing bug.