moxie0/sslstrip

Infinite Redirection problem

Opened this issue · 5 comments

echelonh commented

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 commented

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

echelonh commented

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 commented

@echelonh Could you atleast tell us what you did?

echelonh commented

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

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

@echelonh Thanks, will try that