Closed Bug 1154780 Opened 11 years ago Closed 11 years ago

Users without additional privileges shouldn't be able to revert macros

Categories

(developer.mozilla.org Graveyard :: General, defect)

Product:
developer.mozilla.org Graveyard
Component:
General
Platform:
All
Other
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: fs, Unassigned)

Details

(Keywords: wsec-authorization, Whiteboard: [specification][type:bug])

What did you do? ================ 1. Registered a "normal" new user with no additional permissions. 2. Went to https://developer.mozilla.org/en-US/docs/Template:obsolete_header$revision/786284 3. Clicked on "Revert to this revision" What happened? ============== I was able to revert the macro to an earlier version. What should have happened? ========================== Normal users should not be able to make changes to our macro code. They can break pages that rely on the KumaScripts and affect the whole site. Is there anything else we should know? ====================================== Discovered this through this profile: https://developer.mozilla.org/en-US/profiles/jtuff311 Their first edit is a macro revert.
Flags: needinfo?(lcrouch)
Severity: normal → major
Ouch, yeah. I can't think of a way that a non-super-user could use this to put any users' security or privacy at risk? I.e., all the revisions of the templates are authored by authorized template editors?
Flags: needinfo?(lcrouch)
I'm adding in more checks for template-editing privileges. Do we ever move templates? I can add a check on the move view too.
Flags: needinfo?(fscholz)
We rarely move templates. Not sure I have moved one. Couldn't hurt to ensure proper privileges there, though.
Flags: needinfo?(fscholz)
This bug should have been published to all of the backend devs and wasn't until after it was shipped. This is *not* okay.
(In reply to Jannis Leidel [:jezdez] from comment #5) > This bug should have been published to all of the backend devs and wasn't > until after it was shipped. This is *not* okay. I am sorry, I don't always file security bugs and I was very careful with CCs here. It was, however, not my intend to exclude key people. I made sure Luke and Justin are CC'ed and I assumed they would institute the necessary steps including proper/correct communication.
Agreed; it isn't actually the job of the content team to be sure that exactly the right set of people are cced on bugs. We do our best, but in the end, it comes down the the dev team and its managers to ensure that the correct people are included in the conversation.
The PR in comment 4 was merged. Is this fixed?
Flags: needinfo?(fscholz)
The scenario I described in comment 0 is no longer reproducible on prod. I am not a Kuma engineer, so I will not make the call if this is now secure or not.
Flags: needinfo?(fscholz)
:jcrawford The PR was merged and deployed, so that's some level of "fixed". Marking as resolved. That said, I still think this was mishandled as it wasn't known to me, one of the two backend developers. So please consider this a protest, which is the only thing is left for me to state here now that I don't have a say in it anymore.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
For bugs that are resolved, we remove the security flag. These haven't had their flag removed, so I'm removing it now.
Group: websites-security
Product: developer.mozilla.org → developer.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.