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

CSRF for /manage/delete/#T.Text #2

Closed
Tarrasch opened this issue Jan 6, 2012 · 4 comments
Closed

CSRF for /manage/delete/#T.Text #2

Tarrasch opened this issue Jan 6, 2012 · 4 comments

Comments

@Tarrasch
Copy link
Contributor

Tarrasch commented Jan 6, 2012

Hello, I know it's not at all my business how you run your site, however you might still find interest in this.

If I'm not mistaken if an external site links to say $APPROOT/manage/delete/cool_slug, and one accidentally clicks it and is having an logged in session at your site, then that post would be deleted, no? I'm not an expert on this so correct me if I'm wrong.

I've myself implemented something like your system of documents for my own site. If the issue exists I'm gonna fix it for my own site (and I can show you my commit how I did it).

But knowing you are a lot more experienced with yesod and web, I thought I ask you for advice how you would fix this. Also, CSRF works for /auth/logout. Is that a yesod issue?

So I'm opening this issue to inform and ask for your kind advice on how one could fix this "the yesod way". Do you have any input?

@pbrisbin
Copy link
Owner

pbrisbin commented Jan 6, 2012

Thank you for the interest.

As for post management, I require that the logged in user is also an Admin on the site in order for any of the management pages to load as you can see here.

The userAdmin flag is a field that I manually set through SQL so no one but me should be able to access those links. I haven't tested things as a non-admin user in a long time so please tell me if this isn't working as I'd assumed...

I'm not sure about CSRF on logout, but I simply use the functionality provided by the plugin so perhaps there's a bug there?

@Tarrasch
Copy link
Contributor Author

Tarrasch commented Jan 6, 2012

It's not relevant which users have permission to delete documents, even if you are the only one and ain't gullible. What if you happen to click on http://pbrisbin.com/manage/delete/slug_hehehe? Would that not be a typical CSRF, you click on a link from github and delete your post slug_hehehe on devsite.

You are most likely not worried of this, I know, but if I were to fix this for my own site (or your site), what advice would you give? What do you think is the easiest way?

@pbrisbin
Copy link
Owner

pbrisbin commented Jan 6, 2012

Cross site request forgery? I'm not sure I follow how accidentally deleting my own post would qualify. It's a simple GET request, doesn't matter where I came from.

If my site is somehow open to session hijacking, that someone who isn't me could get my session and delete a post -- that I'd want to hear about!

As for accidents, I'm not worried for the following reasons:

  1. I trust me
  2. I backup my database regularly
  3. The database only holds metadata -- content is in Markdown files on disk (and also backed up)

And if I were, here's how I'd "fix" it:

  1. Make the delete route accept both GET and POST
  2. Make the GET response a confirmation page with a form to submit to the POST route
  3. Make the POST route do the actual deletion.

This would be more in line with convention, standards and would then leverage yesod's built-in CSRF protection via the form-to-POST process as yesod forms include nonce fields by default.

I just didn't think my little site needed such things.

@Tarrasch
Copy link
Contributor Author

Tarrasch commented Jan 6, 2012

Yea, you're completely right in everything, and your "fix" is just how I imagined how I would do it too. Thanks for your input then!

@Tarrasch Tarrasch closed this as completed Jan 6, 2012
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

2 participants