Don't allow GuestView JS objects to inherit global prototype.

Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG=695476
Review-Url: https://codereview.chromium.org/2712913005
Cr-Commit-Position: refs/heads/master@{#452976}
Committed: https://chromium.googlesource.com/chromium/src/+/5934185d281ff83961832317620da5468e7cf703

[wjmaclean, 2017-02-24 21:08:52]

The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run

[wjmaclean, 2017-02-24 21:09:29]

Description was changed from

==========
Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG=695476
==========

to

==========
Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG=695476
==========

[wjmaclean, 2017-02-24 21:09:29]

wjmaclean@chromium.org changed reviewers:
+ lfg@chromium.org

[wjmaclean, 2017-02-24 21:09:45]

lfg@ - ptal?

[commit-bot: I haz the power, 2017-02-24 21:10:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lfg, 2017-02-24 21:11:20]

lfg@chromium.org changed reviewers:
+ lazyboy@chromium.org

[lfg, 2017-02-24 21:11:20]

I'm not comfortable reviewing this, lazyboy@ can you take a look?

[commit-bot: I haz the power, 2017-02-24 21:21:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 21:21:25]

Dry run: This issue passed the CQ dry run.

[lazyboy, 2017-02-24 22:27:57]

lgtm

https://codereview.chromium.org/2712913005/diff/1/extensions/renderer/resourc...
File extensions/renderer/resources/guest_view/guest_view.js (right):

https://codereview.chromium.org/2712913005/diff/1/extensions/renderer/resourc...
extensions/renderer/resources/guest_view/guest_view.js:50:
GuestViewImpl.prototype.__proto__ = null;
A better solution is to use utils.expose() which already does this I think. We
should audit all of guest_view's exposed properties and make sure this kind of
unintended behavior isn't possible to (or at least not straightforward to)
abuse.

It'd be great if you can consider fixing that in subsequent patch(es).

[wjmaclean, 2017-02-24 22:47:58]

On 2017/02/24 22:27:57, lazyboy wrote:
> lgtm
> 
>
https://codereview.chromium.org/2712913005/diff/1/extensions/renderer/resourc...
> File extensions/renderer/resources/guest_view/guest_view.js (right):
> 
>
https://codereview.chromium.org/2712913005/diff/1/extensions/renderer/resourc...
> extensions/renderer/resources/guest_view/guest_view.js:50:
> GuestViewImpl.prototype.__proto__ = null;
> A better solution is to use utils.expose() which already does this I think. We
> should audit all of guest_view's exposed properties and make sure this kind of
> unintended behavior isn't possible to (or at least not straightforward to)
> abuse.
> 
> It'd be great if you can consider fixing that in subsequent patch(es).

I'll land this now, and then implement the improved solution next week, as well
as starting the audit.

[wjmaclean, 2017-02-24 22:48:04]

The CQ bit was checked by wjmaclean@chromium.org

[commit-bot: I haz the power, 2017-02-24 22:48:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 22:59:07]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1487976484104480, "parent_rev":
"b2827d2c7e7fb65a6936bedb3b08409448e4d737", "commit_rev":
"5934185d281ff83961832317620da5468e7cf703"}

[commit-bot: I haz the power, 2017-02-24 22:59:41]

Description was changed from

==========
Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG=695476
==========

to

==========
Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG=695476

Review-Url: https://codereview.chromium.org/2712913005
Cr-Commit-Position: refs/heads/master@{#452976}
Committed:
https://chromium.googlesource.com/chromium/src/+/5934185d281ff83961832317620d...
==========

[commit-bot: I haz the power, 2017-02-24 22:59:42]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/5934185d281ff83961832317620d...