Bug 146040 - Web Inspector: Inspector Scripts evaluated in the page should not be searchable
Summary: Web Inspector: Inspector Scripts evaluated in the page should not be searchable
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
Keywords: InRadar
Depends on:
Reported: 2015-06-16 18:16 PDT by Joseph Pecoraro
Modified: 2015-06-16 22:12 PDT (History)
10 users (show)

See Also:

[PATCH] Proposed Fix (11.73 KB, patch)
2015-06-16 18:28 PDT, Joseph Pecoraro
joepeck: review-
joepeck: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (10.07 KB, patch)
2015-06-16 18:31 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-06-16 18:16:24 PDT
Inspector Scripts evaluated in the page should not be searchable.

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]

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