Skip to content

Fixed open redirect security vulnerability #1383

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

Merged
merged 1 commit into from
Jul 1, 2016
Merged

Conversation

Th3R3p0
Copy link
Contributor

@Th3R3p0 Th3R3p0 commented Jun 30, 2016

The previous filter searched for two forward slashes "//" in the "_next” parameter and if the two forward slashes were found it would check the URI and determine if the hostname matched the hostname of the web server. If the hostname did not match, it would change the "next" variable to the None. However, browsers don't require two forward slashes. As a feature, browsers accept typos such as http:google.com or http:/google.com and redirect to http://google.com. This can be used to leverage an open redirect attack even with the current filter. This commit fixes the open redirect vulnerability in the _next get parameter. Thanks to jnbrex for helping debug/write the patch for this vulnerability.

Note: when testing this vulnerability, you must change the protocol in the URI. For example: if you are hosting the site using http, your redirect must go to a https site. Furthermore: http://web2py.com?_next=https:/malicioussite.com

…hed for two forward slashes "//" in the "_next” parameter and if the two forward slashes were found it would check the URI and determine if the hostname matched the hostname of the web server. If not, it would change the next variable to the None. However, browsers don't require two forward slashes. As a feature, browsers accept typos such as http:google.com or http:/google.com and redirect to http://google.com. This can be used to leverage an open redirect attack even with the current filter. This commit fixes the open redirect vulnerability in the _next get parameter. Thanks to jnbrex for helping debug/write the patch for this vulnerability.
@codecov-io
Copy link

codecov-io commented Jun 30, 2016

Current coverage is 51.22%

Merging #1383 into master will decrease coverage by <.01%

@@             master      #1383   diff @@
==========================================
  Files            43         43          
  Lines         17358      17361     +3   
  Methods           0          0          
  Messages          0          0          
  Branches       4064       4067     +3   
==========================================
  Hits           8894       8894          
  Misses         7396       7396          
- Partials       1068       1071     +3   

Powered by Codecov. Last updated by 5f80300...d95acb6

@Th3R3p0 Th3R3p0 mentioned this pull request Jun 30, 2016
@mdipierro mdipierro merged commit d95acb6 into web2py:master Jul 1, 2016
@mdipierro
Copy link
Contributor

I refactored but should be ok. Thanks for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants