High: several sensitive admin actions bypass sudo re-authentication
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 7k
- Forks
- 476
- Avg merge
- 15h 48m
- Merged PRs (30d)
- 3
Description
Summary
Several high-impact administrative actions appear to require only PRIV_EDIT_SYSTEM but do not enforce requireSudo re-authentication, while other sensitive admin routes in the same file do require sudo confirmation.
This leads to inconsistent protection for dangerous actions such as restart, script execution, and bulk user import.
Impact
If an admin session is stolen, left open, or used in a shared environment, an attacker may be able to:
- restart the service
- execute system scripts
- bulk import accounts and modify groups
without triggering the extra sudo confirmation that is already used elsewhere for sensitive operations.
Code References
Sensitive actions without requireSudo
packages/hydrooj/src/handler/manage.ts:80-89postRestart()executespm2 reload
packages/hydrooj/src/handler/manage.ts:92-131SystemScriptHandler.post()executesglobal.Hydro.script[id].run(...)
packages/hydrooj/src/handler/manage.ts:235-307SystemUserImportHandler.post()performs bulk user creation and group updates
Sensitive actions with requireSudo in the same file
packages/hydrooj/src/handler/manage.ts:134-170- system settings
packages/hydrooj/src/handler/manage.ts:173-231- config editing
packages/hydrooj/src/handler/manage.ts:314-349- user privilege management
Sudo implementation
packages/hydrooj/src/service/server.ts:47-73requireSudodecorator implementation
Why this is a security issue
The project already recognizes the need for step-up authentication through requireSudo, but the protection is applied inconsistently.
Restarting the service, executing system scripts, and bulk importing users are all high-impact actions and should be protected at least as strictly as settings or privilege pages.
Suggested Fix
- Apply
@requireSudoconsistently to all high-impact administrative actions. - Review all
/manage/*handlers for step-up authentication consistency. - Consider documenting a security policy for which actions must require sudo confirmation.
Additional Notes
This report is based on code review of:
packages/hydrooj/src/handler/manage.tspackages/hydrooj/src/service/server.ts
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with packages/hydrooj/src/handler/manage.ts and compare postRestart, SystemScriptHandler.post, and SystemUserImportHandler.post with the protected handlers nearby. Read requireSudo in packages/hydrooj/src/service/server.ts, then review the /manage handlers for consistent step-up protection. Done means the listed high-impact actions and other sensitive management routes enforce the intended sudo confirmation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication, authorization, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 62/100