1235667 - PermissionsUtils.jsm needs to stop using createCodebasePrincipalFromOrigin

Status: RESOLVED WONTFIX

(Firefox :: Security, defect)

Description

[Dave Huseby [:huseby]]

In the file toolkit/modules/PermissionUtils.jsm there's this code:

> 29     for (let origin of origins) {
> 30       let principals = [];
> 31       try {
> 32         principals = [ Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin) ];

Since is related to preferences, which we're not going to isolate by user context id (yet), the solution is to populate an origin attribute from the origin, then create a GlobalContextOriginAttribute to force the user context id to 0, then call createCodebasePrincipal with the origin and the origin attributes instead.

Comment 1

[Tim Huang[:timhuang]]

Attachment: Fix the PermissionsUtils.jsm for using createCodebasePrincipal instead of createCodebasePrincipalFromOrigin.

Comment 2

[Dave Huseby [:huseby]]

So this patch is no longer needed, for the same reasons I stated here: https://bugzilla.mozilla.org/show_bug.cgi?id=1240853#c2

This code is meant to import permissions from a pref.  We don't isolate permissions on user context so propagating origin attributes is not necessary.  The call to:

> Services.perms.addFromPrincipal(principal, aPermission, aAction);

here: http://mzl.la/1UpVFVh ultimately calls nsPermissionManager::AddInternal() (http://mzl.la/1PtZobU) which uses a PermissionKey object to get the hashed string value for isolating the permission.  PermissionKey only uses the origin and not the origin attributes, so there is no reason to add any code for handling the origin attributes.

The bug should be RESOLVED, WON'T FIX

Comment 3

[Dave Huseby [:huseby]]

Comment on attachment 8733793 [details] [diff] [review]
Fix the PermissionsUtils.jsm for using createCodebasePrincipal instead of createCodebasePrincipalFromOrigin.

Review of attachment 8733793 [details] [diff] [review]:
-----------------------------------------------------------------

See https://bugzilla.mozilla.org/show_bug.cgi?id=1235667#c2

Comment 4

[Dave Huseby [:huseby]]

As I pointed out in comment 2, this patch is not needed.