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

Security: Tokens get deleted in certain cases. #838

Closed
binwiederhier opened this issue Aug 17, 2023 · 1 comment
Closed

Security: Tokens get deleted in certain cases. #838

binwiederhier opened this issue Aug 17, 2023 · 1 comment
Labels
🪲 bug Something isn't working 🔒 security Security related ticket

Comments

@binwiederhier
Copy link
Owner

binwiederhier commented Aug 17, 2023

The excess query deletion logic is incorrect and can lead to tokens being deleted erroneously.

Issue

ntfy only allows 20 access tokens to be created per user. Since access tokens are also used by the browser session, excess tokens (> 20) are deleted when the 21st token is created. This excess deletion logic is meant to only delete tokens from the user creating the 21st token. Instead, it accidentally deleted all tokens of all other users, thereby logging everyone out of their ntfy web app sessions, and deleting all other access tokens.

This was a denial-of-service-type security issue, since it effectively allowed a single user to deny access to all other users of a ntfy instance. Please note that while tokens were erroneously deleted, nobody but the token owner ever had access to it.

Details

Original delete query:

DELETE FROM user_token
WHERE (user_id, token) NOT IN (
	SELECT user_id, token
	FROM user_token
	WHERE user_id = ?
	ORDER BY expires DESC
	LIMIT ?
)

Fixed query:

DELETE FROM user_token
WHERE user_id = ?
  AND (user_id, token) NOT IN (
	SELECT user_id, token
	FROM user_token
	WHERE user_id = ?
	ORDER BY expires DESC
	LIMIT ?
)

What to do

If you run a multi-user public system, please update your instances to ntfy v2.7.0

@binwiederhier binwiederhier added the 🪲 bug Something isn't working label Aug 17, 2023
@binwiederhier
Copy link
Owner Author

Fixed in 3e3b556

@binwiederhier binwiederhier added the 🔒 security Security related ticket label Aug 17, 2023
@binwiederhier binwiederhier changed the title Tokens get deleted in certain cases. Security: Tokens get deleted in certain cases. Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working 🔒 security Security related ticket
Projects
None yet
Development

No branches or pull requests

1 participant