Closed Bug 1190233 Opened 4 years ago Closed 4 years ago

"Default" permissions being added to an existing profile without user's consent or awareness

Categories

(SeaMonkey :: Passwords & Permissions, defect)

SeaMonkey 2.36 Branch
defect
Not set

Tracking

(seamonkey2.35 fixed, seamonkey2.36 fixed, seamonkey2.37 fixed, seamonkey2.38 fixed, seamonkey2.39 fixed, seamonkey2.40 fixed)

RESOLVED FIXED
seamonkey2.40
Tracking Status
seamonkey2.35 --- fixed
seamonkey2.36 --- fixed
seamonkey2.37 --- fixed
seamonkey2.38 --- fixed
seamonkey2.39 --- fixed
seamonkey2.40 --- fixed

People

(Reporter: kevink9876543, Assigned: rsx11m.pub)

References

Details

User Story

comm-central (2.40)
http://hg.mozilla.org/comm-central/rev/8b7a22fcff5c
comm-aurora (2.39)
http://hg.mozilla.org/releases/comm-aurora/rev/46921622f2f1
comm-beta (2.38)
http://hg.mozilla.org/releases/comm-beta/rev/f51ce4cdc40d
comm-release (2.37)
http://hg.mozilla.org/releases/comm-release/rev/788a65a99efe
SEAMONKEY_2_36_RELEASE_BRANCH
http://hg.mozilla.org/releases/comm-release/rev/e809326bfcb4
SEAMONKEY_2_35_RELEASE_BRANCH
http://hg.mozilla.org/releases/comm-release/rev/77a191044190

Attachments

(1 file)

(repro directions are for Linux x86_64, but I have seen this in other OSes as well)

Steps to reproduce:
1) Start SeaMonkey 2.33.1 in a new profile
2) Go to about:data, note the permissions
3) Upgrade SeaMonkey to a version based on Firefox 39.0 release or later, say this official Aurora build: https://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2015-08-02-01-30-05-comm-aurora/
4) Start SeaMonkey in the same profile as before, and again note the permissions in about:data


Expected results:
The permissions stay the same.


Actual results:
addons.mozilla.org, marketplace.firefox.com, and downloads.mozdev.org are now suddenly allowed to install add-ons, and support.mozilla.org and input.mozilla.org are given "remote-troubleshooting" permission - all without any signal to the user that any change was made.  Users' existing permissions setups are as they are for a reason, we should be made aware of any change, *especially* a change in the "more permissive" direction, so that we know what our browsers are allowed to do.  And installing add-ons is one of the most significant permissions, as a site allowed to install add-ons is effectively trusted with the privileges of the user running the application.

Possible solutions that I can think of:
 1) Change the default permissions file preference to point to a SeaMonkey-specific file that is a copy of the blank permissions file that was in SeaMonkey 2.33.1, so that there aren't any permissions changes to be made
 2) Throw a confirmation dialog listing the permission change(s) and asking the user whether or not to accept them
 3) Notify the user (perhaps by a doorhanger) that some permission was changed automatically; then if we users care, we go to about:data and find out on our own what changed and deal with it appropriately.


mozillaZine thread: http://forums.mozillazine.org/viewtopic.php?f=40&t=2951459
Bug 1072751 - Switch SeaMonkey from xpinstall.whitelist.add to using a default permissions file.

> Possible solutions that I can think of:
>  1) Change the default permissions file preference to point to a SeaMonkey-
> specific file that is a copy of the blank permissions file that was in 
> SeaMonkey 2.33.1, so that there aren't any permissions changes to be made

That's suite/app/permissions (apparently copy-pasted from its Firefox counterpart).
Blocks: 1072751
As far as I am aware the app/permissions file would only be sourced if there was no permissions database. This would also impact on Firefox, hopefully bsmedberg knows the workings (or knows someone that does).
Flags: needinfo?(benjamin)
As an ad-hoc fix, preferably for 2.35 already, this patch removes the sites which are intended to be used with Firefox only and are thus not relevant for SeaMonkey to start with (specifically the remote-troubleshooting ones users are likely to object allowing those by default). I'm leaving mozdev.org in as it was added on purpose in bug 1072751.
Attachment #8642395 - Flags: review?(iann_bugzilla)
Attachment #8642395 - Flags: feedback?(philip.chee)
(In reply to Ian Neal from comment #2)
> As far as I am aware the app/permissions file would only be sourced if there
> was no permissions database. This would also impact on Firefox, hopefully
> bsmedberg knows the workings (or knows someone that does).

Ian, reports in the MZ thread indicate that these changes were made to /existing/ profiles used in 2.33.1 already, not just in /new/ ones, so that's apparently not the case and the default permissions should be more conservative.
(In reply to rsx11m from comment #4)
> (In reply to Ian Neal from comment #2)
> > As far as I am aware the app/permissions file would only be sourced if there
> > was no permissions database. This would also impact on Firefox, hopefully
> > bsmedberg knows the workings (or knows someone that does).
> 
> Ian, reports in the MZ thread indicate that these changes were made to
> /existing/ profiles used in 2.33.1 already, not just in /new/ ones, so
> that's apparently not the case and the default permissions should be more
> conservative.

Hence the need-info, as it shouldn't be happening and if it is happening I would expect it to impact on Firefox users too.
No extra permissions are added by SeaMonkey 2.35 here, thus 2.35 is actually unaffected by this bug; I've changed the flags accordingly.
(In reply to Ian Neal from comment #2)
> As far as I am aware the app/permissions file would only be sourced if there
> was no permissions database. This would also impact on Firefox, hopefully
> bsmedberg knows the workings (or knows someone that does).

Per bug 1073095 comment 8 and 9, if you don't want default permissions in affect, you should set the permissions url to be empty. I don't think there's a bug here. The sourced prefs are not persisted.
(In reply to Magnus Melin from comment #7)
> Per bug 1073095 comment 8 and 9, if you don't want default permissions in
> affect, you should set the permissions url to be empty. I don't think
> there's a bug here.
That's fine for single user setups (I'll go do that right now in my user.js).  The bug here is not simply that changes are being made to users' permissions (existing or new), it's that users who don't compulsively check about:data have no idea that their existing permissions have been altered in the "more permissive" direction.
Comment on attachment 8642395 [details] [diff] [review]
Mitigating fix removing SM-unrelated sites

Ayup. My bad. r=me CLOSED TREE
Attachment #8642395 - Flags: feedback?(philip.chee) → review+
Comment on attachment 8642395 [details] [diff] [review]
Mitigating fix removing SM-unrelated sites

Since Phil gave the patch an r+ already, cancelling r? for IanN, but requesting branch approval for 2.36-2.38 (do we want this for 2.35 anyway?).

[Approval Request Comment]
Regression caused by (bug #): bug 1072751
User impact if declined: sites apparently useless for SM given default permissions
Testing completed (on m-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: none
Attachment #8642395 - Flags: review?(iann_bugzilla)
Attachment #8642395 - Flags: approval-comm-release?
Attachment #8642395 - Flags: approval-comm-beta?
Attachment #8642395 - Flags: approval-comm-aurora?
Keywords: checkin-needed
Whiteboard: [c-n: comm-central][leave open]
I don't think you need any more information from me, but in case you do these are default permissions loaded at each startup which can be overridden by user permissions, but they aren't loaded into the user permission database. This is similar to the way default prefs work.
Flags: needinfo?(benjamin)
Comment on attachment 8642395 [details] [diff] [review]
Mitigating fix removing SM-unrelated sites

a=me for 2.36 and above (as far as I am aware 2.35 is unaffected)
Attachment #8642395 - Flags: approval-comm-release?
Attachment #8642395 - Flags: approval-comm-beta?
Attachment #8642395 - Flags: approval-comm-beta+
Attachment #8642395 - Flags: approval-comm-aurora?
Attachment #8642395 - Flags: approval-comm-aurora+
Comment on attachment 8642395 [details] [diff] [review]
Mitigating fix removing SM-unrelated sites

[Triage Comment]
a=me for 2.36 too, wrong item selected
Attachment #8642395 - Flags: approval-comm-release+
Thanks, push for all default channels please.
Whiteboard: [c-n: comm-central][leave open] → [c-n: comm-central/aurora/beta/release default branches][leave open]
a=me for 2.35 as pointed out the original patch landed on 2.35
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Whiteboard: [c-n: comm-central/aurora/beta/release default branches][leave open] → [c-n: comm-central/aurora/beta, comm-release default/sm2.35 branch][leave open]
Keywords: checkin-needed
Whiteboard: [c-n: comm-central/aurora/beta, comm-release default/sm2.35 branch][leave open] → [leave open]
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Ratty: Thanks for the heavy branch work!  :-)

kevink/barbaz: This bug has been closed after the patch has been checked into all branches. If you think that anything from your list in the opening description should be considered and implemented, please open a follow-up bug. Given that (1) has been implemented here with reducing the whitelist to match effectively what was allowed in 2.33.1 (with the exception of mozdev having been added), note that both (2) and (3) from your list require string changes, thus won't be able to land on the branches (and thus in time for when the change from bug 1072751 affects the user).
(In reply to rsx11m from comment #18)
> kevink/barbaz: This bug has been closed after the patch has been checked
> into all branches. If you think that anything from your list in the opening
> description should be considered and implemented, please open a follow-up
> bug. Given that (1) has been implemented here with reducing the whitelist to
> match effectively what was allowed in 2.33.1 (with the exception of mozdev
> having been added), note that both (2) and (3) from your list require string
> changes, thus won't be able to land on the branches (and thus in time for
> when the change from bug 1072751 affects the user).
With the change that was committed, implementing (2) or (3) is far from urgent atm IMO.

Thanks all for taking care of this. :)
You need to log in before you can comment on or make changes to this bug.