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

Hash of the password stored in the cookies #2470

Closed
jvoisin opened this issue Dec 21, 2020 · 6 comments
Closed

Hash of the password stored in the cookies #2470

jvoisin opened this issue Dec 21, 2020 · 6 comments
Assignees
@ainar-g @EugeneOne1
Labels
bug P2: High
Milestone
v0.105.2

Comments

@jvoisin
Copy link
Contributor

jvoisin commented Dec 21, 2020

It seems that the username as well as the bcrypt'ed password have their sha256 stored in the cookie.

This is a bit worrying, since an attacker able to get the cookie would not only gain access to AGH, but also be able to bruteforce offline the password. A possible way to address this would be to use a random string instead of sha256(salt | username | bcrypt(password)).
@stale
Copy link

stale bot commented Feb 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 21, 2021
@jvoisin
Copy link
Contributor Author

jvoisin commented Feb 26, 2021

Shall I ask for a CVE for this?

@stale stale bot removed the wontfix label Feb 26, 2021
@ameshkov
Copy link
Member

Huh, sorry, no idea how this report was missed. We'll address it in v0.105.2

@ameshkov ameshkov added this to the v0.105.2 milestone Feb 26, 2021
@ameshkov ameshkov added the bug label Feb 26, 2021
adguard pushed a commit that referenced this issue Mar 1, 2021
@EugeneOne1
Pull request: 2470 session token
94e783d 
Merge in DNS/adguard-home from 2470-session-token to master

Updates #2470.

Squashed commit of the following:

commit 02e8744
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Mar 1 20:11:35 2021 +0300

    home: imp time formating

commit 6f4a6c9
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Mar 1 18:48:15 2021 +0300

    home: rm user's data from session token
@EugeneOne1
Copy link
Member

The solution you proposed is implemented in snapshot 94e783d. All new cookies provided by AGH now have 128-bit random string as the value and don't contain any user's information. Could you please verify if it's secure enough for you?

@jvoisin
Copy link
Contributor Author

jvoisin commented Mar 2, 2021

Looks good to me, thanks!

@jvoisin jvoisin closed this as completed Mar 2, 2021
@jvoisin
Copy link
Contributor Author

jvoisin commented Mar 3, 2021

This has been assigned CVE-2021-27935

heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
@EugeneOne1 @heyxkhoa
Pull request: 2470 session token
4450e73 
Merge in DNS/adguard-home from 2470-session-token to master

Updates AdguardTeam#2470.

Squashed commit of the following:

commit 02e8744
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Mar 1 20:11:35 2021 +0300

    home: imp time formating

commit 6f4a6c9
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Mar 1 18:48:15 2021 +0300

    home: rm user's data from session token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ainar-g ainar-g

@EugeneOne1 EugeneOne1

Labels
bug P2: High
Projects
None yet
Milestone
v0.105.2
Development

No branches or pull requests
4 participants
@jvoisin @ainar-g @ameshkov @EugeneOne1