Fix permissions issues on main.queries and users.queries. #4400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @corentin-soriano, it looks good to me, the fixes have resolved the vulnerabilities I reported. |
nilsteampassnet
approved these changes
Oct 8, 2024
nilsteampassnet
approved these changes
Oct 8, 2024
nilsteampassnet
approved these changes
Oct 8, 2024
| $data_user = DB::queryfirstrow( | ||
| 'SELECT admin, gestionnaire, can_manage_all_users, isAdministratedByRole FROM ' . prefixTable('users') . ' | ||
| WHERE email = %s', | ||
| $post_id |
There was a problem hiding this comment.
$post_id isn't initialized at this step
There was a problem hiding this comment.
I forgot to replace $post_id with $dataReceived['receipt']
| // Manager of basic/ro users in this role but don't allow promote user to admin or managers roles | ||
| || ((int) $session->get('user-manager') === 1 | ||
| && in_array($data_user['isAdministratedByRole'], $session->get('user-roles_array')) | ||
| && (int) $post_is_admin !== 1 && (int) $data_user['admin'] !== 1 |
There was a problem hiding this comment.
Same for $post_is_admin, $post_is_hr and $post_is_manager
There was a problem hiding this comment.
Copy/paste error.
We must check if the query returns a result and if the user has the right to administer the account found.
I will correct this tomorrow.
| // Use id from session if user not admin or id not sent. | ||
| $userId = $session->get('user-id'); | ||
| } | ||
|
|
||
| // Handle the case where no PWD is provided (user reset his own encryption keys). | ||
| if (empty($dataReceived['user_pwd']) && (int) $userId === $session->get('user-id')) { |
There was a problem hiding this comment.
$userId is not initialized
There was a problem hiding this comment.
$filtered_user is expected.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
$session->get('user-id')instead of user input when only the user himself can perform this action.initialize_user_passwordremoved because it is unused.@tvnnn can you test this branch please?