[GitHub] [trafficserver] neex opened a new issue #7563: cache key collision because of username URL decode in url_CryptoHash_get_general

[GitBox <gi...@apache.org>]

neex opened a new issue #7563:
URL: https://github.com/apache/trafficserver/issues/7563


   The code in `url_CryptoHash_get_general` at URL.cc decodes percent encoding in all components of the URL, including username / password. This behaviour is incorrect and can lead to security consequences.
   
   Consider the following scenario:
   
   1. An ATS instance serves two domains: `attacker.com` and `victim.com`. This can happen either in forward-proxy mode or in the reverse proxy mode in some cases.
   2. An attacker controls the content of `attacker.com` (e.g. can serve whatever she wants to ATS when ATS makes requests to attacker.com, as well as to instruct ATS to cache this content).
   3. The attacker wants to put her content into the context of `victim.com` to achieve e.g. XSS.
   
   The attacker can visit `http://%3A%40victim.com%2F@attacker.com/` via ATS (note the encoded characters in the username). She can do so by sending the link in the HTTP request, like this:
   
   ```
   GET http://%3A%40victim.com%2F@attacker.com/ HTTP/1.1
   ...
   ```
   
   ATS will forward this request to `attacker.com`, but the key in the cache (calculated in url_CryptoHash_get_general before hashing) will be `http://:@victim.com/:@attacker.com/`. Now, if someone visits `http://victim.com/:@attacker.com/` via ATS, it will lookup the response in the cache and serve the attacker's page, thus achieving XSS impact (the attacker can send this link to someone and execute javascript in the context of `victim.com`).
   
   The same impact can be achieved using MD5 collisions. The cache key is hashed using MD5, and there're public tools to create an MD5 collision (including chosen-prefix ones).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] SolidWallOfCode commented on issue #7563: cache key collision because of username URL decode in url_CryptoHash_get_general

[GitBox <gi...@apache.org>]

SolidWallOfCode commented on issue #7563:
URL: https://github.com/apache/trafficserver/issues/7563#issuecomment-790998349


   I agree, we shouldn't URI decode the user name/password.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] github-actions[bot] commented on issue #7563: cache key collision because of username URL decode in url_CryptoHash_get_general

[GitBox <gi...@apache.org>]

github-actions[bot] commented on issue #7563:
URL: https://github.com/apache/trafficserver/issues/7563#issuecomment-1059874684


   This issue has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] SolidWallOfCode commented on issue #7563: cache key collision because of username URL decode in url_CryptoHash_get_general

[GitBox <gi...@apache.org>]

SolidWallOfCode commented on issue #7563:
URL: https://github.com/apache/trafficserver/issues/7563#issuecomment-791013602


   Unfortunately, it looks appears to be spec legal to use percent encoding in the user name. I need to think about this a bit more.
   
   MD5 collisions aren't a concern - the cache already deals with it. Internally the stripe directory is the equivalent of an open chained hash and the MD5 value is used only to locate the bucket. The full URL is used for equality checks.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] neex commented on issue #7563: cache key collision because of username URL decode in url_CryptoHash_get_general

[GitBox <gi...@apache.org>]

neex commented on issue #7563:
URL: https://github.com/apache/trafficserver/issues/7563#issuecomment-791365042


   I think the "fair" solution would be to encode back some characters (e.g. `/`, `:`', `@` and so on) so that if one of these appears in the username/password, it will always be in its encoded form.
   
   However, note that several (but less security impactful) issues can arise in the other part of the URL. E.g. in some hypothetical example, we can register a user with the name `victim&page=posts` and then get a page like `index.php?username=victim%26page=posts` cached as if it were `index.php?username=victim&page=posts` and still get cache poisoning (but inside the same domain).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org