Skip to content
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

Infinite Redirection problem #12

Open
echelonh opened this issue Feb 3, 2014 · 5 comments
Open

Infinite Redirection problem #12

echelonh opened this issue Feb 3, 2014 · 5 comments

Comments

@echelonh
Copy link

echelonh commented Feb 3, 2014

Hi there!

While using sslstrip, I encountered a rather small but crucial issue of infinite redirections.

Some links get redirected to "http://www.evil.com" while others might redirect to "http://www.evil.com/". A small difference, but an important one.

Since the '/' char is sometimes missing, the function "addSecureLink" cannot really find the index of "pathIndex", hence its value is set to -1 and "path" turns to be the full url ("http://www.evil.com").

The problem comes up next when ClientRequest uses UrlMonitor's "IsSecureLink" to decide how to treat the HTTP Request. "handleHostResolvedSuccess" passes the url to "IsSecureLink" with a '/' char (I didn't find out the reason exactly, but it does), and so the check if "(client, url) in self.strippedURLs" fails since
http://www.evil.com != http://www.evil.com/

This might lead to all sorts of unexpected behaviors, in my case - an infinite redirection.

I fixed this bug by checking for pathIndex value, and adding a '/' if necessary. Hopefully this bug will be fixed in the master branch as well :).

Thanks you Moxie for this tool :)!

@cmavr8
Copy link

cmavr8 commented Feb 4, 2014

Great to see people helping this project!
Did you fork and implement your fix? Maybe make a pull request?

@echelonh
Copy link
Author

echelonh commented Feb 6, 2014

I just pulled the source code and fixed it locally, I haven't tried to add my patch to the master branch, as I wasn't sure about permissions, and branching, and quite honestly I'm pretty new to this whole git thing.

If you want to help me with it - I'll be more than happy to do it.

@martijnboers
Copy link

@echelonh Could you atleast tell us what you did?

@echelonh
Copy link
Author

echelonh commented Apr 8, 2014

@MartijnDevNull Of course!
URLMonitor.py, line 54 - Replace with:

    pathIndex   = url.find("/", methodIndex)
    if (pathIndex == -1):
        pathIndex = len(url)
        url += "/"

@martijnboers
Copy link

@echelonh Thanks, will try that

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

No branches or pull requests

3 participants