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.

Sign up for GitHub

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

Jump to bottom

Bug in the identity impersonalization part of the security package #36174

Closed
AllanFousse opened this issue Mar 23, 2020 · 4 comments
Closed

Bug in the identity impersonalization part of the security package #36174

AllanFousse opened this issue Mar 23, 2020 · 4 comments
Labels
Bug Security Status: Needs Review

Comments

@AllanFousse
Copy link

AllanFousse commented Mar 23, 2020 •
edited

Symfony version(s) affected: 5.0.5

Description

Impersonalisation not working if we use Uuid for username in Postgresql database

How to reproduce

use uuid type instead string in postgresql database for username

Possible Solution
generate à uuid string instead of pure random string line 152 of SwitchUserListener.php

Additional context
the error is SQLSTATE[22P02]: Invalid text representation
@fancyweb fancyweb mentioned this issue Mar 26, 2020
Merged
@fancyweb
Copy link
Contributor

Thanks for reporting @AllanFousse. What is the exact exception you get please?

@AllanFousse
Copy link
Author

AllanFousse commented Mar 26, 2020 •
edited

Annotation 2020-03-26 184850

is that you need ?

@fancyweb
Copy link
Contributor

Yes, thank you. Can you check #36223 please?

@AllanFousse
Copy link
Author

I try the modification, and it's work fine for me

nicolas-grekas added a commit that referenced this issue Apr 1, 2020
@nicolas-grekas
bug #36223 [Security][Http][SwitchUserListener] Ignore all non existe…
15edfd3 
…nt username protection errors (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security][Http][SwitchUserListener] Ignore all non existent username protection errors

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #36174
| License       | MIT
| Doc PR        | -

Since we generate the non existent username blindly, it can lead to Doctrine exceptions or any other exception.

We can catch all exceptions here but I guess it reduces the protection since the SQL query was not executed?

Alternative: we can only catch Doctrine DriverException (in addition to the existing AuthenticationException) and only silent the reported error codes?

Commits
-------

42311d5 [Security][Http][SwitchUserListener] Ignore all non existent username protection errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @xabbuh @fancyweb @AllanFousse @carsonbot