Bug 151134 - JS builtins should use secured functions
Summary: JS builtins should use secured functions
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-11 06:49 PST by Xabier Rodríguez Calvar
Modified: 2015-12-14 21:06 PST (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xabier Rodríguez Calvar 2015-11-11 06:49:42 PST
As we saw in bug 151104, bug 151033 and bug 150895 and bug 150934, there's builtin code that can be easily disrupted by users. We might want to find a solution for this.
Comment 1 Yusuke Suzuki 2015-11-11 07:59:05 PST
For InternalPromise, easy way I think is, freezing an internal promise instance.
But one concern is performance...
Comment 2 youenn fablet 2015-11-11 23:39:26 PST
I wonder how much of these unsafe cases can be flagged using static analysis/style checking tools.

Do we have a JS style checker?
Would it be also useful for the inspector code?
Comment 3 Xabier Rodríguez Calvar 2015-11-12 01:08:00 PST
(In reply to comment #2)
> I wonder how much of these unsafe cases can be flagged using static
> analysis/style checking tools.
> 
> Do we have a JS style checker?
> Would it be also useful for the inspector code?

I don't know if a style checker is enough. It might be a step forward, but from my POV there are tons of unsafe operations. Take an array as an example. We use array operations all the way long at the builtins. A malicious change in the array prototype could be devastating (I haven't tried, though). Yes, we could do something similar to what we did with the promises at streams and use internal prototype operations but the more you do this, the more you make your code absolutely unreadable.

I think the situation should be solved in a different way that could be marking builtins code with a secure flag that would make some operations safe by default preventing any disruption from the user's world.
Comment 4 youenn fablet 2015-11-12 02:47:01 PST
Ultimately, we might want to run built-in scripts in an isolated world with separate DOM prototypes and constructors.

We would be able to use them on DOM C++ instances (which would have a JS wrapper for user scripts and a JS wrapper for built-in scripts). Cost might be ok?
We would then be able to write JS code much more cleanly than we are now.

One issue though is for pure JS objects, like promises and streams.
I am not sure how we can handle these properly and efficiently.
Comment 6 Xabier Rodríguez Calvar 2015-11-12 04:29:13 PST
(In reply to comment #4)
> Ultimately, we might want to run built-in scripts in an isolated world with
> separate DOM prototypes and constructors.
> 
> We would be able to use them on DOM C++ instances (which would have a JS
> wrapper for user scripts and a JS wrapper for built-in scripts). Cost might
> be ok?
> We would then be able to write JS code much more cleanly than we are now.
> 
> One issue though is for pure JS objects, like promises and streams.
> I am not sure how we can handle these properly and efficiently.

I thought of using worlds too, but the problem with worlds is that you have to keep a link with the user world because there are some things that come from there, say user promises, etc. and even more, there are some things that could be generated in an isolated world that should be returned to the user world.
Comment 7 Xabier Rodríguez Calvar 2015-11-23 11:13:49 PST
This discussions seems to be a bit stalled. I am tempted to move it to the ML.
Comment 8 youenn fablet 2015-11-24 01:11:09 PST
Here are a bunch of additional thoughts.

In terms of coding style, it seems easier to secure JS built-ins when most of the code meat is implemented as internal functions and not as methods of the prototype or constructor. Whenever there is a need to access to a particular functionality, the internal function may be called in lieu of the object method.

For objects that are never exposed to user scripts, private methods of their prototypes are safe, public methods are not of course. There are two questions though: first, how can we ensure that these objects are never exposed by accident? second, how can we ensure that the JS built-in code is not calling public methods of these objects?
If JS Proxy was available in WebKit, we could wrap these objects as proxies in debug mode (using something like @assert(this.@myvar = Proxy(...)).
The proxies would be responsible to verify how these objects are accessed.

For objects that are exposed to user scripts, private methods of their prototype may not be available since the object prototype may have been changed. In that case, a solution is to do something like @Constructor.prototype.@method.@call. Far from perfect in terms of readability...
Again, for those objects, we could use Proxy to ensure they are safely accessed (@assert(stream = Proxy(stream...)).
We could also try sanitizing somehow these objects, for instance at the time promises are returned to streams from user scripts, or wrapping them.

Also, there are two different type of threats.
First a user script may disrupt an API by tweaking objects and prototypes.
This happens in the case where a JS built-in is expected to access a private method stored in the prototype.
If the user changes the prototype, the private method will not be available, thus breaking the functionality.
This is not a very good story but this is not too evil really.

Second, a user script may get access to data that should not be visible to user scripts.
This may happen for instance if an object is using a public method, the object or its prototype being accidentally exposed to user scripts. This is really bad.
To limit the second threat, using private methods should be recommended and conversely, using public methods should be discouraged in JS built-ins, even if the object/prototype is not supposed to get exposed.
Access to public methods stored in prototypes is particularly problematic since if one object is exposed, its prototype will also be exposed and this may end-up leaking information for all objects related to that prototype.