Clarification needed for V3.1.1 Verify the application never reveals session tokens in URL parameters or error messages. #696
Comments
elarlang
changed the title
|
So regarding problem 2, when I think of this requirement, I always think of: https://auth0.com/docs/security/bulletins/cve-2019-7644 :) Regarding 1, have you ever seen a session token in a URL path? Seems a little odd, no? |
|
|
|
This is to avoid session url rewriting which in turn can lead to session fixation
And in general never putting any kind of sensitive data in any url (even over https) is a good practice
…--
Jim Manico
@manicode
Secure Coding Education
+1 (808) 652-3805
On Dec 22, 2019, at 2:49 PM, Josh Grossman ***@***.***> wrote:
OK, I would be open to a PR with a suggestion.
Saying you should not return session token in HTTP response is a bit too general I think, can you think of a more specific way of wording?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
hey @elarlang do you have any thoughts for a PR for 1) and more specific wording for 2) |
tghosth
added
the
2) Awaiting response
but this part is duplicate for V8.3.1 (if we just say clearly that session token IS sensitive piece of information)
this is not clear or understandable for me, why to limit it only to error messages. What attack vector it takes away?
So, for me it's not clear, what is the goal for this sentence and that's why I can not propose any PR yet. In general - if we remove this "error message" part, then the requirement is "in the URL is not ok but in message body is ok" which also not true. In practice, maybe we need to reorganise the idea for this requirement with V3.2.3.
It has the same problem - use correct settings for cookie or use HTML5 session storage. And we reach to one setting - is HttpOnly flag for cookie important or not. In other words - should session token be readable for JavaScript or not. Should we have this extra mitigation for "XSS situations" or not. If you have XSS on site... your active session is usable for attacker via your browser anyway. I think, we need to check separately "Cookie-based" and "Token-based" session management for those requirements. If we have "Cookie-based Session management"
If we have "Token-based Session management"
And then there are solutions, where Token is used in cookie... where we should have question, do you use token-based session management correctly. My current conclusions/ideas:
|
|
How about...
... as part of a URL in any HTTP verb ...
Even sensitive data in the URL target of a post or put request is a bad idea.
…--
Jim Manico
@manicode
Secure Coding Education
+1 (808) 652-3805
On May 9, 2020, at 4:34 PM, Elar Lang ***@***.***> wrote:
For presenting session token in URL would be probably something like:
V3.1.1 Verify the application never reveals session tokens as part of URL
it covers
session token is not leaked by default to logs, to browser history etc
potential referrer problem (but it should be covered with Referrer-Policy requirement)
but this part is duplicate for V8.3.1 (if we just say clearly that session token IS sensitive piece of information)
V8.3.1 Verify that sensitive data is sent to the server in the HTTP message body or headers, and that query string parameters from any HTTP verb do not contain sensitive data.
... or error messages
this is not clear or understandable for me, why to limit it only to error messages. What attack vector it takes away?
if the point is - session token should not be presented in error messages, it's covered by V7.1.1
if the point is - session token should not be presented to the output at all, then limiting to "error messages" does not make sense
for example, to just display it to the output is ok by this requirement
So, for me it's not clear, what is the goal for this sentence and that's why I can not propose any PR yet.
In general - if we remove this "error message" part, then the requirement is "in the URL is not ok but in message body is ok" which also not true.
In practice, maybe we need to reorganise the idea for this requirement with V3.2.3.
V3.2.3 Verify the application only stores session tokens in the browser using secure methods such as appropriately secured cookies (see section 3.4) or HTML 5 session storage.
It has the same problem - use correct settings for cookie or use HTML5 session storage. And we reach to one setting - is HttpOnly flag for cookie important or not. In other words - should session token be readable for JavaScript or not. Should we have this extra mitigation for "XSS situations" or not. If you have XSS on site... your active session is usable for attacker via your browser anyway.
I think, we need to check separately "Cookie-based" and "Token-based" session management for those requirements.
If we have "Cookie-based Session management"
V3.1.1 should have meaning "do not use session token anywhere else then httponly flagged cookie"
V3.2.3 should not allow using session token in session storage in this case
If we have "Token-based Session management"
V3.1.1 should have meaning "do not spread session token to URL"
but if the value used by JavaScript with extra header (like Authorization) for API it is anyway readable for JavaScript, do you display it to the output does not even matter in practice
meaning is covered by V8.3.1
V3.2.3 basically says, store it in session storage. But here is another conflict - if we consider session token as sensitive data, then it does not match with V8.2.2
V8.2.2 Verify that data stored in client side storage (such as HTML5 local storage, session storage, IndexedDB, regular cookies or Flash cookies) does not contain sensitive data or PII.
And then there are solutions, where Token is used in cookie... where we should have question, do you use token-based session management correctly.
My current conclusions/ideas:
I don't see point for current V3.1.1
if needed, modify "V8.3.1 Verify that sensitive data (including session token) ..."
I don't see point for current V3.2.3
Add new requirement to Cookie-based Session Management with meaning "session token is allowed to send only with httponly cookies" (but isn't it too strict?)
If token-based session should be able to use cookies, move current V3.2.3 to Token-based requirement. If token-based session should not use cookie, use only "session storage part" from V3.2.3
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Then it's clear duplicate for V8.3.1. And we still have 2nd part of the requirement. |
tghosth
removed
the
2) Awaiting response
|
@jmanico, please take your time to investigate arguments above and let's find some proposal/decision for this. About 2nd part of the requirement:
For example, if site displays somewhere session tokens (e.g. Why to limit in the requirement, that session token can not leak only through error messages? It does not make sense for me. If session is cookie-based, it should have |
|
Oh absolutely this requirement makes no sense and needs fixing. Good call, thank you.
…--
Jim Manico
@manicode
On Jun 10, 2020, at 6:04 AM, Elar Lang ***@***.***> wrote:
@jmanico, please take your time to investigate arguments above and let's find some proposal/decision for this.
About 2nd part of the requirement:
V3.1.1 Verify the application never reveals session tokens in URL parameters or error messages.
For example, if site displays somewhere somewhere session tokens (e.g. phpinfo() output, list of active sessions etc), then those outputs are readable in XSS situation for JavaScript - session token leakage, potential session hijacking.
Why to limit in the requirement, that session token can not leak only through error messages? It does not make sense for me. If session is cookie-based, it should have httpOnly flag set and never displayed to any part of HTTP response other than Set-Cookie header.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
As long as we reword, I can see this being in 4.0.2, rather than waiting for 4.1 |
|
I want to make sure session fixation is addressed and that url rewriting is specifically not in play, so I simplified this requirement and am closing it out. |
|
V3.1.1 got emergency fix, but I would like to keep this issue open for v4.1 to address ideas/proposals from #696 (comment) |
|
I will reopen but I think that comment needs to be a little clearer or at least highlight the conclusion if it is the tl;dr |
jmanico
changed the title
|
What needs to happen so we can close this out? |
Discussion 1 - V3.1.1 vs V8.3.1V3.1.1 is covered by V8.3.1, but maybe it's good to keep this as clear separate requirement for sessions.
Discussion 2 - cookie based session managementCurrent V3.2.3 will be removed one way or another (by issue #843 or like I recommended already here #696 (comment)) If cookie-based session management is used, should we have requirement (like I proposed in #696 (comment)): "Verify that cookie-based session tokens are only transferred in Set-Cookie and Cookie headers" |
|
Thank you @elarlang for clearing up the complex issues we're discussing. You're on it. |
|
531 looks the closest https://cwe.mitre.org/data/definitions/531.html Or maybe https://cwe.mitre.org/data/definitions/489.html None look like an exact match |
|
CWE-200 might be a good fit. |
|
Proposal:
|
|
This seems quite reasonable. Can we add "and to specifically avoid sending session data via url?" Thats the real worry. |
No :)
|
|
Booooo! |
cookie-based tokens should stay in cookies (closes #696)
|
Few comments for "future investigations" and improvements:
Added requirement:
If there is
|
Current requirement:
Problem 1: URL parameters
It's kind of points to GET parameters. If session token is presented as "folder", it's still the same problem.
Recommendation:
"part of URL" ?
Problem 2: error messages
I can not understand the reason here (it should be more clear).
Should this / or separate requirement cover also situation, if application sends session tokens to 3rd party program (like sama user-interaction trackers) as plain-text?
The text was updated successfully, but these errors were encountered: