Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin: admins can usurp owners #1586

Closed
HumorBaby opened this issue May 6, 2019 · 1 comment
Closed

admin: admins can usurp owners #1586

HumorBaby opened this issue May 6, 2019 · 1 comment
Labels
Bug Things to squish; generally used for issues
Milestone

Comments

@HumorBaby
Copy link
Contributor

I believe admin and owner are meant to be two separate permission levels for the bot. There are @require_admin and @require_owner for commands, implying that there are certain functions for an owner that should not be available to admins.

That being said, if the admin plugin is enabled, an admin can simply:

Not a spoiler 😅
.set core.owner I-Am-gRoot
bypassing the @require_owner, and potentially locking them out of their own bot.
@dgw dgw added the Bug Things to squish; generally used for issues label May 6, 2019
@dgw dgw modified the milestones: 6.6.7, 6.6.8 May 6, 2019
@dgw
Copy link
Member

dgw commented May 6, 2019

Good ol' privilege escalation vulnerabilities… Where would we be without them?

The obvious, easy solution is to hard-code into admin.py a block on changing this specific setting if the user isn't owner. I'd like not to do that, though. I already don't like the hard-coded logic around preventing the output of passwords when getting existing setting values.

Buuuuuut, fixing it another way would involve API-breaking. That is, settings would somehow need a way to reject being changed by lowly admins (or at all) at runtime, and that would require changing the object model (I think). Fine for Sopel 8—as I try to avoid adding anything more to the overstuffed Sopel 7 milestone 😆—but not something we can shove into a 6.6.x patch release.

Don't really want to leave this backdoor open until 8.x happens, though. That's over a year off.

How about fixing it the easy way for 6.6.x (as now milestoned)? We can open a follow-up issue to design a settings API change after that's done and released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Development

No branches or pull requests

2 participants