Menu

#15 csrf token not generated with an empty session...

open
nobody
None
5
2012-08-29
2012-07-15
No

I was walking through the forms tutorial and enabled a <% csrf %> token. The <% csrf %> token is an empty string unless you set a value in the session. Calling session().save() doesn't generate a session either, but manually assigning a value via session()["foo"] = "bar";, works. I believe this is a bug. If there is a <% csrf %> tag, there is an implicit understanding that a _csrf value should be generated even though no session has been explicitly created and assigned to the user.

I'm not 100% clear on the next item, but it also looks like if a cookie is sent back to the server and it's missing a _csrf value, it won't generate a new csrf token.

http://cppcms.com/wikipp/en/page/cppcms_1x_forms

If you need a test-case, I can post my example, however I think the stock example suffers from this problem.

The output of a <% csrf %> tag via telnet(1) is:

<input type="hidden" name="_csrf" value="">

Discussion

  • Artyom Beilis

    Artyom Beilis - 2012-07-15

    This is by design.

    It is mean-less to put CSRF token to protect a user that is not "logged-in". Also CSRF token is session specific and if you don't have a session you don't need one.

    Bottom line: CSRF protection is meaning less in context of "Guest users" as "request forgery" of "non-logged-in" user does not have sense.

     
  • Sean Chittenden

    Sean Chittenden - 2012-07-15

    The entire class of Login CSRF attacks exist because login forms are not wrapped in CSRF protection. It seems like it's much easier to just return a CSRF token under all circumstances.

    Look for the white paper "Robust Defenses for Cross-Site Request Forgery" if you're interested in some more background and details.

    http://www.adambarth.com/papers/2008/barth-jackson-mitchell-b.pdf

     
  • Artyom Beilis

    Artyom Beilis - 2012-07-15

    I see, thanks for the link.

    It seems like it's much easier to just return a CSRF token under all circumstances.

    It is by no means easier because:

    • Create session for EVERY page that contains any form (because csrf token by design is session specific).
    • I do not know in advance if the page that is rendered contains a form at all - so I need to create a session for EVERY page.
    • It makes caching much harder...

    Anything you need to do to prevent CSRF logic attack is to create session before login form.

    void login()
    {
    if(!session().is_set("prelogin")) {
    session.reset();
    session.set("prelogin","");
    response().set_redirect_header(url("."));
    return;
    }

    }

    I think it is good idea to add CSRF login attack to this lost together with:

    http://cppcms.com/wikipp/en/page/secure_programming#Session.Fixation

    I need to think about.

     
  • Sean Chittenden

    Sean Chittenden - 2012-07-15

    nods It's a hard problem, especially since I don't think CppCMS buffers its output before sending the response. The "easy" fix is to have the <% csrf %> token generate a token and then inform the response object that a session cookie needs to be set. Is there a way to change the function signature of the render() method for pages that have a csrf token? I was thinking the easiest way would be to have a render() and render_buffered() where the buffered render call could interact with the response() object before the rendered template gets sent out. ? I don't know if that's correct, I'm still learning the code, but that's the first thing that came to mind.

    Lastly, if you're worried about session fixation, it's not possible to address session fixation without the server maintaining state (i.e. client is always going to be vulnerable to fixation). A hijacked and stolen cookie is inherently replayable. session.reset() must invalidate the trust relationship on the server-side because the server doesn't know if the session information has leaked to a third party. If you're interested, I have examples of how I handled this in some python Flask code using PostgreSQL.

    Logout

    https://github.com/sean-/flask-skeleton/blob/develop/sql/initialize/300_funcs.sql.in#L239

    Login (requires a session token)

    https://github.com/sean-/flask-skeleton/blob/develop/sql/initialize/300_funcs.sql.in#L73

    Expire sessions:

    https://github.com/sean-/flask-skeleton/blob/develop/sql/initialize/300_funcs.sql.in#L337

    Trigger to expire old sessions:

    https://github.com/sean-/flask-skeleton/blob/develop/sql/initialize/300_funcs.sql.in#L373

    This is a more involved implementation example than what's minimally required, but a session token handed to a client and only a client is vulnerable to replay. If a client session_id is a proxy token that represents a state entry on the server-side, then you can guard against fixation by revoking the trust between the server-side state and the client's session_id.

     
  • Anonymous

    Anonymous - 2012-07-16

    I understand the implementation difficulties - and i think the extra work required for login forms is not that much, and does not merit the addition of caching into the CppCMS equation
    i think that the csrf implementation should remain as it is now - but that this subject will be documented a lot clearer both in the csrf documentantation and in the tutorial

     
  • Sean Chittenden

    Sean Chittenden - 2012-07-16

    Good enough. I haven't tested, but won't the logic demonstrated result in an infinite loop if the browser is rejecting cookies?

    Here's a small editorialized version of the updated paragraph:

    Remember: this protection depends on session validation, so in order to protect a form, a session must exists when the page is rendered (even for forms that have no other need for session values - like login forms). To protect the login method from Login CSRF attacks, the login() method should look something similar to this (this application pattern should be applied to all forms where a session isn't present upon the initial rendering of a form):

     
  • Artyom Beilis

    Artyom Beilis - 2012-07-16

    The "easy" fix is to have the <% csrf %> token generate a token and then inform the response object that a session cookie needs to be set.

    The problem is that once output generated the session information is already saved (think about client side session storage). So you can't add csrf token once HTTP headers are left.

    About session fixation.

    When the session is stored on the client side, it is not a problem as upon login it would be updated
    so the attackers cookie would be invalid. resest session is required to change session id when the session
    is stored on the server side.

    In any case session fixation implementation requires XSS vulnerability.

    In general after few thoughts I think this pattern is the best

    void login()
    {
    login_content c;

    if(request().request_method() == "POST" && session().is_set().("prelogin")) {
        c.myform.load(context());
        if(c.myform(validate() && correct_login(myform)) {
          session().reset_session();
          session().erase("prelogin");
          session().set("id",some_id_I_created);
          response().set_redirect_header("/some/path");
          return;
        }
    }
    session.set("prelogin","");
    render("loginform",c);
    

    }

    So on GET method or on a method where sessions is empty session is created and then on POST
    it validated

    i.e. all I need in my login form is following:

    1. Change
      if(request().request_method()=="POST") {
      to
      if(request().request_method()=="POST" && session().is_set("prelogin")) {

    2, Change

       session().reset_session();
    

    To

       session().reset_session();
       session().erase("prelogin");
    
    1. If don't pass login ensure that the session is created using "prelogin" after if(request()... } { } session.set("prelogin","")
     
  • Artyom Beilis

    Artyom Beilis - 2012-07-16

    Cool.

    Thanks nice. I also added a simple example of Login CSRF attack to make it much
    more clear how it works.

    Now what I think it is good idea to add following functions to session_interface class:

    // make sure that session exists even without some arbitrary tokens
    void create();
    // Check if session does not exits at all (empty)
    bool is_empty();

     
  • Artyom Beilis

    Artyom Beilis - 2012-07-16

    (But this is for next version)

    Thanks for the valuable input!

     
  • Sean Chittenden

    Sean Chittenden - 2012-07-16

    My pleasure, and yes, the updated docs look much better! Thank you both.

     

Anonymous
Anonymous

Add attachments
Cancel





Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.