Roundup Tracker - Issues

Issue 2550690

classification
Title: Inadequate CSRF protection
Type: security Severity: normal
Components: Web interface Versions: 1.4
process
Status: new Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ber, eadler, joseph_myers, rouilj, schlatterbeck
Priority: high Keywords:

Created on 2011-02-22 20:15 by joseph_myers, last changed 2016-07-13 00:22 by rouilj.

Messages
msg4246 Author: [hidden] (joseph_myers) Date: 2011-02-22 20:15
Roundup's web interface checks that requests modifying the database come
via HTTP POST.  This is not sufficient protection against cross-site
request forgery attacks; a user's browser can easily be induced by a
hostile site to send an arbitrary POST request, through Javascript or
otherwise.

In addition to the POST check, all those actions checking for POST
should also check for a secret token in a hidden form field, linked in
the database to the session ID.  (Because of incompatibility with
existing templates it may not be possible to do this unconditionally,
but I think security against CSRF should be the default.)

The POST check is still useful, as it makes it unlikely for the secret
token to appear in URLs (since it wouldn't work there) and URLs are
liable to leak (in Referers etc.).  It's useful for the secret token to
be a separate random token rather than a copy of the session ID, so that
if the contents of a form are leaked (e.g. if the user sends of copy of
the page to someone) that doesn't by itself enable impersonating the user.
msg4786 Author: [hidden] (ber) Date: 2013-02-04 09:43
Joseph, thanks for creating the issue.
Can you point to a good explanation how this protection can work?
Thanks,
Bernhard
msg4788 Author: [hidden] (schlatterbeck) Date: 2013-02-04 10:13
On Mon, Feb 04, 2013 at 09:43:44AM +0000, Bernhard Reiter wrote:

Thanks for putting me on the nosy list.

I think I can offer some guidance on how to implement this but currently
don't have too much time myself...

And I agree this should be high prio.

Ralf
msg4790 Author: [hidden] (joseph_myers) Date: 2013-02-04 14:01
https://en.wikipedia.org/wiki/Cross-site_request_forgery#Prevention
https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet
(for example) discuss various approaches used to protect against CSRF.  
For Roundup I still think the right approach is a secret token used in all 
forms (and it should not be possible to compute the session ID from this 
token, so that having a leaked copy of a form doesn't allow a third party 
to generate a cookie to impersonate the user).
msg4791 Author: [hidden] (ber) Date: 2013-02-04 14:53
Joseph, thanks for the hints!
msg4935 Author: [hidden] (rouilj) Date: 2013-10-09 20:31
Also note that the CSRF token may need to be a single use token.
So as soon as a post is made with the token, it gets masked/rewritten
and is no longer valid.

This protects again BREACH types of attacks against the token.

http://breachattack.com/

which recommends the following mitigation methods:

    Disabling HTTP compression

    Separating secrets from user input

    Randomizing secrets per request

    Masking secrets (effectively randomizing by XORing with a random
secret per request)

    Protecting vulnerable pages with CSRF

    Length hiding (by adding random number of bytes to the responses)

    Rate-limiting the requests
msg5843 Author: [hidden] (rouilj) Date: 2016-07-13 00:22
I wonder if we could use the one time key mechanism used for password
resets here as well.

For each form we generate a template function that produces:

  <hidden name="otk" value="....." >

and change RetireAction, RestoreAction, EditCSVAction,
EditItemAction::handler, NewItemAction::handler, RegisterActon::hander,
(maybe SearchAction) to verify the otk against what is stored
int he session database.

Would this basic idea work?
History
Date User Action Args
2016-07-13 00:22:33rouiljsetmessages: + msg5843
2013-10-09 20:31:12rouiljsetnosy: + rouilj
messages: + msg4935
2013-04-08 22:07:19eadlersetnosy: + eadler
2013-02-04 14:53:18bersetmessages: + msg4791
2013-02-04 14:01:29joseph_myerssetmessages: + msg4790
2013-02-04 10:13:25schlatterbecksetmessages: + msg4788
2013-02-04 09:43:44bersetpriority: high
nosy: + schlatterbeck, ber
messages: + msg4786
2011-02-22 20:15:53joseph_myerscreate