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

Grant should not be included for db user #209

Open
rezalas opened this issue Oct 19, 2023 · 6 comments
Open

Grant should not be included for db user #209

rezalas opened this issue Oct 19, 2023 · 6 comments
Labels
security security flaw related bug

Comments

@rezalas
Copy link
Owner

rezalas commented Oct 19, 2023

We should remove grant as a general perm to avoid potential permission leaks here. The DB user has no actual need for grant and really any non-CRUD related function, including alter.

mysql -uhomestead -psecret -e "CREATE USER IF NOT EXISTS '$USER'@'localhost' IDENTIFIED BY '$PASS';"
mysql -uhomestead -psecret -e "GRANT ALTER,CREATE,DELETE,DROP,INDEX,INSERT,SELECT,UPDATE ON rift.* TO '$USER'@'localhost';"
mysql -uhomestead -psecret -e "GRANT ALTER,CREATE,DELETE,DROP,INDEX,INSERT,SELECT,UPDATE ON rift_core.* TO '$USER'@'localhost';"

@sean-gilliam
Copy link
Collaborator

Most definitely.

@rezalas rezalas added the security security flaw related bug label Oct 19, 2023
@Psypher9
Copy link
Collaborator

Isn't this just granting the permissions in the comma separated list to the user? Not granting GRANT. Perhaps I'm misunderstanding.

@sean-gilliam
Copy link
Collaborator

I just looked at the three line snippet and thought a user supplied USER was being given those perms. So I took a look at the whole file to get context.

It's pretty clear that the script is only giving those perms to the rift user. That's the exact same stuff that the setup scripts/admin stuff uses. This looks like a script to run in case of an oopsie when the rift user gets deleted somehow.

@Psypher9
Copy link
Collaborator

Yeah that's what I was thinking too. So are we thinking just leave it?

@rezalas
Copy link
Owner Author

rezalas commented Oct 20, 2023

I read it wrong initially, but we still need to pull alter and drop off the list just in case someone gets an injection and calls them.

Alternatively, we could migrate it to a dev setup only. The production setup though should have the absolute minimum rights needed to operate for the game.

@Psypher9
Copy link
Collaborator

Okay I see what you're saying now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security security flaw related bug
Projects
None yet
Development

No branches or pull requests

3 participants