icecoder / ICEcoder

Browser code editor awesomeness
http://icecoder.net
Other
1.42k stars 347 forks source link

CSRF error #390

Closed ghost closed 10 years ago

ghost commented 10 years ago

Hi, I followed all the instructions on how to install ICEcoder. But as soon as I navigate to the directory for the first time. I get a "CSRF error". I can't do anything at that point, there is no log in screen, nothing. "The CSRF token could not be verified" Is this a server related problem? Or a bug? Version 3.5 is working great. But I would love to try 4.0

I hope you can help, Thanks

ghost commented 10 years ago

Hi everyone,

I uninstalled Yii framework, and all of a sudden the CSRF error disappeared. This might be good to know for other people with the same problem. I did a few Google searches, and it says that Yii framework has a CSRF protection module. So that must have been it.

Thanks guys! It's working :-)

mattpass commented 10 years ago

Hey Tim, thanks for letting me know about this.

I introduced CSRF protection last week after a security review from the people over at Bugcrowd. Think I have all the code right and this is maybe a clash in CSRF naming conventions between ICEcoder and Yii (probably both using the name 'csrf')? I don't know much about Yii, but that's a possibility anyway.

Will look into it and perhaps come up with a more unique name (maybe 'ICEcsrf') to avoid any clashes with other systems/frameworks.

Thanks again and glad you found a workaround anyways.

Will leave this issue open as a reminder to myself to look into it.

ghost commented 10 years ago

Thank you for your response Matt! I'm glad I could help identify it. But I should be thanking you, for all your hard work. ICEcoder is great, you did an amazing job. Thank you! Tim

codersteve commented 10 years ago

Hi Tim,

I came across ICEcoder tonight and I have to say: it's Awesome! You have done some fantastic work!

I also came across the CSRF error after I installed ICEcoder on my shared host.

However, after I replaced the single IF statement (containing the CSRF error alert) in /ICEcoder/lib/headers.php with the nested IF statements shown below, the login page loaded normally and I was able to sign in and start using the editor without any problems.

Can you confirm my modified code (notably use of isset($_REQUEST["csrf"])) is equivalent to what you wanted the original IF statement to do?

Thanks for all your hard work!

Cheers Steve J.

if (isset($_REQUEST["csrf"])) {
    if($_REQUEST["csrf"] != $_SESSION["csrf"]) {
        echo '<script>alert("Bad CSRF token. Please press F12, view the console and report the error, including file & line number, so it can be fixed. Many thanks!");</script>';
        echo '<script>console.log("CSRF issue: REQUEST: "+$_REQUEST["csrf"]+", SESSION: "+$_SESSION["csrf"]);</script>';
        die('Bad CSRF token');
    }
}
mattpass commented 10 years ago

Hi Steve

It's actually me that created ICEcoder, rather than Tim. ;-) Glad you like it tho.

You're right, it will be that if condition that needs changing. I think this is a little better tho and would like to know if this works for you:

if (($_GET || $_POST) && (!isset($_REQUEST["csrf"]) || $_REQUEST["csrf"] !== $_SESSION["csrf"])) {

The reason I prefer this is that it will die with the error message if we have GET or POST data and we have no CSRF token or one that doesn't match.

Part of the reason here I'm thinking this is better, is that I'm going through some security improvements right now and would rather see something break with a CSRF error (so I know about it and can add CSRF checking to it), than have a GET or POST submission that isn't CSRF checked and go unnoticed.

Hope that makes sense - all thoughts welcomed.

Matt

codersteve commented 10 years ago

Hi Matt,

Apologies for getting you confused with Tim!!

You are doing some fantastic work with this project, keep it up!!

I agree that your new single IF statement is much better. It certainly seems to work too!

I have made your change to my install of ICEcoder on the server.

Have quickly tested it in Chrome, Firefox and Explorer and the mydomain.com/ICEcoder/ URL still loads the login page like it should.

Hopefully this means this bug is now fixed?? :)

Cheers Steve

mattpass commented 10 years ago

I'd agree it can probably be closed.

Would just like to know if it works for Tim also and then can definitely clear this issue.

codersteve commented 10 years ago

I should probably point out I'm currently only able to test it with http not https.

Is Tim able to test with https ?

mattpass commented 10 years ago

I think this issue can be closed, if anyone has any further CSRF issues please reopen. Thanks!