Bug 33616 – Inspector interface allows insecure data transfer

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Geoffrey Garen]

From https://bugs.webkit.org/show_bug.cgi?id=33469:

> 2. I don't think interfaces like this are safe:
> 
> +    ScriptObject injectedScriptFor(ScriptState*);
> +    ScriptObject injectedScriptForId(long);
> 
> Because you're giving out a generic object, the client of the interface can do
> anything with the object, including passing it unsafe data. It would be better
> if the C++ object wrapping the "injected script" controlled all interaction
> with it, to ensure that no non-primitive data leaked across the boundary.
Eventually we will create a typed interface for that object. Currently it is
used only as ScriptFunctionCall argument but we should restrict the set of
functions that can be called on that object, it would make the injected script
interface clearer. We've already done similar thing in InspectorFrontend.cpp

[Yury Semikhatsky]

Created attachment 48019 [details]
patch

[Pavel Feldman]

Comment on attachment 48019 [details]
patch

> +    ScriptFunctionCall function(scriptState, m_injectedScriptObject, "getCallFrames");

"getCallFrames" -> "callFrames" ?

> +    ScriptValue callFramesValue = function.call();
> +    return callFramesValue.toString(scriptState);
> +}
> +
> +String InjectedScript::wrapAndStringify(ScriptValue value)
> +{
> +    ASSERT(!hasNoValue());
> +    ScriptState* scriptState = m_injectedScriptObject.scriptState();
> +    ScriptFunctionCall wrapFunction(scriptState, m_injectedScriptObject, "wrapAndStringifyObject");
> +    wrapFunction.appendArgument(value);
> +    wrapFunction.appendArgument("console");

Should this be reflected in the method name? wrapAndStringifyForConsole?

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 48019 [details])
> > +    ScriptFunctionCall function(scriptState, m_injectedScriptObject, "getCallFrames");
> 
> "getCallFrames" -> "callFrames" ?
> 
Done.

> > +    ScriptValue callFramesValue = function.call();
> > +    return callFramesValue.toString(scriptState);
> > +}
> > +
> > +String InjectedScript::wrapAndStringify(ScriptValue value)
> > +{
> > +    ASSERT(!hasNoValue());
> > +    ScriptState* scriptState = m_injectedScriptObject.scriptState();
> > +    ScriptFunctionCall wrapFunction(scriptState, m_injectedScriptObject, "wrapAndStringifyObject");
> > +    wrapFunction.appendArgument(value);
> > +    wrapFunction.appendArgument("console");
> 
> Should this be reflected in the method name? wrapAndStringifyForConsole?
Done.


Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/bindings/js/JSInjectedScriptHostCustom.cpp
	M	WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp
	A	WebCore/inspector/InjectedScript.cpp
	A	WebCore/inspector/InjectedScript.h
	M	WebCore/inspector/InjectedScriptHost.cpp
	M	WebCore/inspector/InjectedScriptHost.h
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/front-end/InjectedScript.js
Committed r54285

[Yury Semikhatsky]

Reverted because it broke compilation on Linux GTK.

[Yury Semikhatsky]

(In reply to comment #4)
> Reverted because it broke compilation on Linux GTK.

Committed r54334 with the new files added to WebCore/GNUmakefile.am