Skip to content

Fix privilege escalation in profile update - #34

Open
sxyrxyy wants to merge 1 commit into
ionsec:mainfrom
sxyrxyy:fix/profile-mass-assignment-privilege-escalation
Open

Fix privilege escalation in profile update#34
sxyrxyy wants to merge 1 commit into
ionsec:mainfrom
sxyrxyy:fix/profile-mass-assignment-privilege-escalation

Conversation

@sxyrxyy

@sxyrxyy sxyrxyy commented Jul 10, 2026

Copy link
Copy Markdown

What

PUT /api/user/profile passed the whole request body into User.update, and User.update's field whitelist still included role, organizationId, isActive and permissions. Any logged-in user could set role: "super_admin" on their own account in a single request, then sail through every requireSuperAdmin() / requirePermission('canManageAllUsers') check.

With self-registration on, that's an unauthenticated path to full super_admin over the whole tenant set.

Fix

  • Build an explicit allowlist of profile fields at the route (firstName, lastName, phone, bio, etc.) instead of passing req.body straight through.
  • Drop role, organizationId, isActive and permissions from User.update's whitelist so they can't be set via the self-service path even if a caller tries. Role and permission changes already have dedicated admin endpoints.

Repro (before)

PUT /api/user/profile HTTP/1.1
Authorization: Bearer <any valid JWT>
Content-Type: application/json

{"role":"super_admin","permissions":{"canManageAllUsers":true}}

The caller's own row becomes role = super_admin. After this PR the same request has no effect on role.

Notes

  • Two files, no behavior change for legitimate profile edits.
  • I didn't run the test suite locally - worth a quick pass on the profile-update tests before merge.

PUT /api/user/profile handed the whole request body to User.update, and User.update's whitelist still had role/organizationId/isActive/permissions in it. Any logged-in user could set role=super_admin on their own account.

Allowlist the profile fields at the route and drop the privilege columns from User.update. Role and permissions already have their own admin endpoints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant