-
Notifications
You must be signed in to change notification settings - Fork 14
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
Redis backend doesn't regenerate iron _session_id #12
Comments
I don't really understand how this is vulnerable to session fixation. Could you
elaborate?
…On Sat, Jul 22, 2017 at 01:05:13PM +0000, asakasa wrote:
Even though a value of `iron_session_id` in cookie is **empty**, SessionStorage doesn't generate new session id.
Browser sends a cookie if value is not set. In this situation, SessionStorage generates a new session id and send a header `Set-Cookie: ...`. But SessionStorage uses an empty session id as valid, and then store some values in associating with an empty key. It is buggy I think.
Additionally, if a value of `iron_session_id` isn't empty, Calling `set` method (ex. `req.session.set(T)`) in web application, store some values in associating with the value of `iron_session_id`. It may be correct. But there is a vulnerable to attack "**session fixation**" potentially, so it is fear to use this library for user authorization.
FYI, this is document about Session fixation: [https://www.owasp.org/index.php/Session_fixation](https://www.owasp.org/index.php/Session_fixation)
A better solution I think is that SessionStorage also provides a method to use session id that be specified by Web application.
Thanks for the awesome library.
#### Version of iron-sessionstorage
`iron-sessionstorage = {version="0.6.6", features=["redis-backend"]}`
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#12
|
Sorry for the late reply. If there is only vulnerability to attack session fixation, it is not too dangerous. The vulnerability, however, is too dangerous when other vulnerabilities are combined with it. The flow example of how the attacker exploits is as follows:
Of course, the presence of other vulnerabilities are inherently a critical flaw. In many web services, a session is regenerated when an user logged in. So, how about this library also providing such a method? Otherwise, if it is empty it will not be regenerated, so I think it's better to fix it preferentially. Thanks. |
Would you consider it sufficient if clear() generated a new session?
Otherwise patches welcome, I generally agree.
…On 31 July 2017 20:34:52 GMT+02:00, asakasa ***@***.***> wrote:
Sorry for the late reply.
If there is only vulnerability to attack session fixation, it is not
too dangerous. The vulnerability, however, is too dangerous when other
vulnerabilities are combined with it.
The flow example of how the attacker exploits is as follows:
1. Attacker fixes malicious session to the victim's browser using some
other vulnerabilities, such as XSS or HTTP header injection, etc. For
example, the attacker fixes "**iron_session_id=malicious**" to the
victims's browser.
2. The victim will log in to the service with the malicious session
(**iron_session_id=malicious**).
3. Web service using RedisBackend stores the malicious session in
associating with the victim authentication.
4. Attacker log in to the service using the victim's session.
Of course, the presence of other vulnerabilities are inherently a
critical flaw.
In many web services, a session is regenerated when an user logged in.
So, how about this library also providing such a method?
---
Otherwise, if it is empty it will not be regenerated, so I think it's
better to fix it preferentially.
[https://github.com/iron/iron-sessionstorage/blob/master/src/backends/redis.rs#L93-L97](https://github.com/iron/iron-sessionstorage/blob/master/src/backends/redis.rs#L93-L97)
Thanks.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#12 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Ah, right. I didn't have that idea. Surely it is sufficient for bug fix. Even if you fix the bug using such a way, the vulnerability of Session Fixation is not gone. I think a countermeasure you suggested is good for temporary response. Thanks. |
I think it would make sense to automatically refresh the session if it is cleared, but yes, a new API for this would be good. |
Even though a value of
iron_session_id
in cookie is empty, SessionStorage doesn't generate new session id.Browser sends a cookie if value is not set. In this situation, SessionStorage generates a new session id and send a header
Set-Cookie: ...
. But SessionStorage uses an empty session id as valid, and then store some values in associating with an empty key. It is buggy I think.Additionally, if a value of
iron_session_id
isn't empty, Callingset
method (ex.req.session.set(T)
) in web application, store some values in associating with the value ofiron_session_id
. It may be correct. But there is a vulnerable to attack "session fixation" potentially, so it is fear to use this library for user authorization.FYI, this is document about Session fixation: https://www.owasp.org/index.php/Session_fixation
A better solution I think is that SessionStorage also provides a method to use session id that be specified by Web application.
Thanks for the awesome library.
Version of iron-sessionstorage
iron-sessionstorage = {version="0.6.6", features=["redis-backend"]}
The text was updated successfully, but these errors were encountered: