[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squirrelmail-devel
Subject:    Re: [SM-DEVEL] Countering Cross Site Request Forgeries (XSRF)
From:       Paul Lesneiwski <paul () squirrelmail ! org>
Date:       2005-10-11 22:15:37
Message-ID: 434C3989.80408 () squirrelmail ! org
[Download RAW message or body]

>>I want to proof SquirrelMail against Cross Site Request Forgeries
>>(XSRF / CSRF). A summary is here and the links at the bottom provide
>>very good overviews of the issue.
>>http://en.wikipedia.org/wiki/Cross-site_request_forgery
>>
>>
>>In short, it's possible to have a form on some site and submit it to a
>>squirrelmail script running on a different site, where the latter accepts
>>it just like it was his own form. A very feasible attack would be for an
>>attacker to send email from this user's account.
>>
>>What we need to do is make sure that each submitted form verifies that
>>it was submitted from within SquirrelMail. BTW, when I talk about a form
>>from now on, I mean a form that does something potentially harmfull. The
>>compose form would qualify, the search form would not.
>>
>>My proposed solution would do the following with the compose form as an
>>example:
>>- the user accesses the form in question
>>- we set some kind of secret in the session and send the form with the
>>secret in a hidden field - the user fills out and submits the form
>>- the receiving script checks the secret against the session.
>>- if it does not match: the user gets an error and the action aborts
>>- if it matches, the action continues as usual
>>
>>
>>The implementation would be like this:
>>
>>
>>// called in the code that generates the HTML for the form, somewhere
>>// within <form> and </form>
>>function addFormCheck() { $val = generatesomerandomstring();
>>echo addHidden('formkey', $val); $_SESSION['FORM_KEYS'][] = $val;
>>}
>>
>>
>>// called at the start of the code that processes the form.
>>// if everything ok, calling this function will do nothing and execution
>>// continues.
>>function checkForm() { if ( ! in_array( $_POST['formkey'],
>>$_SESSION['FORM_KEYS'] ) {
>>error_box(_("The integrity of this form could not be verified. Please
>>access the form again and retry.") ); exit; }
>>// cleanup:
>>// delete that value from the array. or better use an indexed array
>>// (map) to allow for quicker checking/deleting?
>>}
>>
>>
>>
>>The implementation is quite straightforward. It works when the user has
>>two different forms open at the same time. If the user repeatedly accesses
>>some form without submitting it (this could be the case for e.g. forms
>>that are a part of a larger page with other content), the array could grow
>>and grow, in theory. Not sure whether that's a realistic problem.
>>
> 
> 
> Every compose form already have an unique id called composesession in
> order to keep track which uploaded attachments belongs to which compose
> form.
> Try to use that id when you store the unique generated form_key.
> 
> The session storage will be something like this:
> $_SESSION['FORM_KEYS'][$composesession] = $some_unique_id;

I tried brieftly to implement this scheme some months ago, maybe even as
a plugin (can't remember).  After fiddling about for a while, it became
apparent that it was more involved than I had the time/patience for at
that time, since I was generating a random key somewhere in the compose
code and I guess testing for the code in the compose_send hook.... my
testers quickly found that there were other ways to send a message (I
think the listcommands plugin was one; bug report would likely be
another...) that hadn't spawned a code in the session, so they were
booted even though the form submission was legit.

In fact, there are several, even many, plugins that send messages in one
way or another, so it would probably be important to take them and the
many permutations that they introduce into account whilst coding this.

 - paul





-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
--
squirrelmail-devel mailing list
Posting Guidelines: http://squirrelmail.org/wiki/wiki.php?MailingListPostingGuidelines
List Address: squirrelmail-devel@lists.sourceforge.net
List Archives: http://news.gmane.org/thread.php?group=gmane.mail.squirrelmail.devel
List Archives: http://sourceforge.net/mailarchive/forum.php?forum_id=7139
List Info: https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel
[prev in list] [next in list] [prev in thread] [next in thread]