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

Jump to bottom

RootPage.loginUser can lead to sensitive information leak #1063

Closed
rcaa opened this issue Apr 18, 2016 · 2 comments
Closed

RootPage.loginUser can lead to sensitive information leak #1063

rcaa opened this issue Apr 18, 2016 · 2 comments
Labels
Catg-Enhancement
Milestone
1.9.0

Comments

@rcaa
Copy link

@rcaa rcaa commented Apr 18, 2016

RootPage.app().authentication().setCookie(request, response, user);

The user argument contains sensitive information, such as user password. The user password is combined with user login to form a hash using SHA-1 algorithm. However, this is a weak algorithm, and tools like hash killer can easily decrypt billions of hashes.

I would suggest avoiding passing user password to setCookie(). This would help to prevent someone accessing our accounts when they have access to our browser cookies.
@gitblit
Copy link
Owner

@gitblit gitblit commented Apr 20, 2016

Hi @rcaa. Thanks for the report. Perhaps you'd be willing to submit a PR?

@rcaa
Copy link
Author

@rcaa rcaa commented Apr 25, 2016

Sure, @gitblit . I have submitted a pull request just now.

@ubala ubala mentioned this issue May 13, 2016
Closed
9 tasks
flaix added a commit to flaix/gitblit that referenced this issue Dec 10, 2016
@flaix
Use a SecureRandom instead of Math.random for user cookie.
0e3e47b 
To get proper entropy in user authentication cookie creation,
make use of `SecureRandom` instead of using `Math.random()`, or
`Random`.

Introduce our own wrapper `SecureRandom` around `java.security.SecureRandom`.
This a) makes sure that the PRNG is seeded on creation and not when
random bytes are retrieved, and
b) uses a static instance in the `UserModel` so that lags do not occur
during operation due to potentially seeding getting blocked on Unix
when reading from the system's entropy pool. To keep the random data
still secure, the static instance will reseed all 24 hours, also a
functionality of the wrapper class.

This fixes gitblit#1063 and extends and closes PR gitblit#1116
@flaix flaix added this to the 1.9.0 milestone Dec 13, 2016
@flaix flaix closed this as completed in 2be2c2c Dec 13, 2016
@flaix flaix added Status-Done Catg-Enhancement Status-Obsolete and removed Status-Done labels Dec 13, 2016
@flaix flaix removed the Status-Obsolete label Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Catg-Enhancement
Projects
None yet
Milestone
1.9.0
Development

No branches or pull requests
3 participants
@gitblit @rcaa @flaix