Defect #3351
Weak autologin token generation algorithm causes duplicate tokens
| Status: | Closed | Start date: | 2009-05-13 | |
|---|---|---|---|---|
| Priority: | Urgent | Due date: | ||
| Assignee: | - | % Done: | 0% | |
| Category: | Accounts / authentication | |||
| Target version: | 0.8.4 | |||
| Resolution: | Fixed | Affected version: |
Description
After switching to mod_passenger we got 7 (seven!) duplicated autologin tokens within 2 weeks. It caused some changes have been made under wrong user account!
Looks like due to using of pseudo-random sequence generator two concurrent Ruby processes may use the same random seed (and as result the same random sequence).
At our instance we made quick fix - prepend random sequence with "#{user.id}_" and substring left 40 chars, however, I guess there may be better solution.
Associated revisions
Use ActiveSupport::SecureRandom to generate tokens (#3351).
Add token value uniqueness validation (#3351).
Do not autologin if more that one token is found (#3351).
History
#1
Updated by Alexander Pavlov almost 12 years ago
Also, I suggest to deny login if search by autologin within Token table returned 2 or more records - it allows to prevent and troubleshot possible errors in future.
#2
Updated by Jean-Philippe Lang almost 12 years ago
- Status changed from New to Resolved
- Target version set to 0.8.4
- Resolution set to Fixed
- ActiveSupport::SecureRandom is now used to generate tokens
- Added a validation on token uniqueness that will prevent 2 tokens with the same value from being saved
- Autologin is denied if more than one token is found
#3
Updated by Alexander Pavlov almost 12 years ago
I never experienced
We suspect it is due to process forking which leads to random sequence seed inherited from parent process so two processes continue working with the same sequence.
ActiveSupport::SecureRandom is now used to generate tokens
Thanks, it is what we were going to suggest!
#4
Updated by Alexander Pavlov almost 12 years ago
Small example from our developers
irb(main):004:0> rand(50)
=> 9
irb(main):005:0> fork { puts rand(50) }
37
=> 22831
irb(main):006:0> rand 50
=> 37
#5
Updated by Alexander Pavlov almost 12 years ago
Also, you could check your DB to ensure you have really never affected by this vulnerability
select value, count(*) from tokens group by value having count(*) > 1
#6
Updated by Jean-Philippe Lang almost 12 years ago
That's what I did when I said that I never experienced this issue.
#7
Updated by Alexander Pavlov almost 12 years ago
Jean-Philippe Lang wrote:
That's what I did when I said that I never experienced this issue.
Probably you are not using mod_passenger. If you started several predefined processes (without mod_passenger) then random sequence had their own seeds and their own random sequences.
mod_passenger do forks and these inherit parent seed (see post #4) - it is key factor to reproduce problem.
#8
Updated by Jean-Philippe Lang almost 12 years ago
- Status changed from Resolved to Closed
Indeed, I'm using apache+mod_fcgid.
Fixes are backported in 0.8-stable branch in r2747.