csrf – jforum cleanup and problems

See part 1 for how we got here  and part 2 for how we changed the OWASP filter.

Code cleanup and problems

There is some poorly written code in JForum that CSRF now prevents from working.  In these cases, I needed to clean up our code.  For example:

  1. Links/anchors shouldn’t be used to update state.  They should only be used for gets.  The OWASP filter is realistic enough to recognize links are in fact used for updates in the real world and accomodates them.  It doesn’t accommodate code like
    onclick="document.location = '${contextPath}/${jforumMainServletMapping}/${moduleName}/...';" />

    In the cases this was used for actions that required CSRF protection, I needed to refactor to use a form.  In a few cases, I created a one to one form.  On others where there were a lot of actions or it wasn’t conducive, I used a common form

    <!-- CSRF token messes up submission if forum id isn't in URL -->
    
    function submitActionForForum(actionVerb, forumId) {
    
    var action = document.actionForm.action;
    
    action = action.replace("ACTION", actionVerb);
    
    action = action.replace("FORUM_ID", forumId);
    
    document.actionForm.action = action;
    
    document.actionForm.submit();
    
    }
    
    </script>
    
    <#-- general purpose form for the submit buttons on the screen; see JS for ACTION and FORUM_ID values -->
    
    <form method="post" name="actionForm" action="${contextPath}/${jforumMainServletMapping}/${moduleName}/ACTION/FORUM_ID">
    
    </form>
    
    ...
    
    onClick="submitActionForForum('up', ${forum.id})"
    
  2. Some pages (like the first one I happened to test) didn’t have </head> so dynamically adding the JavaScript which sets the CSRF token wasn’t working.  I wrote a unit test to identify such files and ensure we don’t create any more.  We also added the missing HTML headers.  Most of them were on the admin pages.
JForum changes
JForum has some features that don’t play well with OWASP CSRF Guard
  1. JForum’s controller framework is heavily dependent on the number of query parameters . Adding a CSRF token broke all sorts of things.  I solved this by adding a check that the parameter wasn’t the CSRF token to the loops in WebRequestContext’s constructor, parseFriendlyURL()  and handleMultipart().  The one in handleMultiPart is unnecessary and just there for consistency.  I also have the constructor calling a method to see if the query string is empty instead of a one liner:
    private boolean isQueryStringEmpty(HttpServletRequest superRequest) {
    String queryString = superRequest.getQueryString();
    
    if (queryString == null || queryString.length() == 0) {
    
    return true;
    
    }
    
    // ignore OWASP token (so if only OWASP present, ignore it)
    
    return queryString.matches("^&?" + CsrfFilter.OWASP_CSRF_TOKEN_NAME + "=[-0-9a-zA-Z]+$");
    
    }
  2. I then learned that multipart reads the input stream and can’t be called twice.  One approach would be to copy the stream.  I didn’t go that route.  The only multipart requests are posts and PMs.  Both of which need CSRF protection.  Instead I have the filter set to just assume multi part requests need CSRF protection.
  3. Added <noscript> on post_form.htm to explain that JavaScript is now required to post.
  4. A couple places were missing <form> around input fields.
  5. In one place, a jQuery form needed the token set explicitly since it was choosing the parameters for jQuery to send on.  (I think this was only in our JForum fork.)
  6. The CSRF filter requires AJAX to pass the token has a header not a parameter.  To add it to a jquery.ajax block:
    headers: {'OWASP_CSRFTOKEN': '${OWASP_CSRFTOKEN!""}'},
    

    This syntax uses Freemarker’s default value to use a space instead of blowing up if there is no token.

Note to users
I was worried about users seeing the CSRF screen.  Some users won’t know what CSRF is.  And most wouldn’t know what to do.  Feel free to read our error page which includes things to try and who to email.
Enhancements/Longer term goals
We started these but it will take a while
  1. Don’t use token for anonymous users
  2. Get rid of CSRF Token from the  URLs (use forms consistently for posts)
  3. Rollback any hacks we made to fix things/cleanup code.
  4. Reconsider using JavaScript to add the tokens.

Note: all code is sanitized to remove references to javaranch/coderanch.

 

The story continues in part 4 – deciding not to use JavaScript for the solution.

2 thoughts on “csrf – jforum cleanup and problems

  1. Pingback: fixing csrf for jforum and csrf filter analysis (part 1) | Down Home Country Coding With Scott Selikoff and Jeanne Boyarsky

  2. Pingback: blogging from owasp security meetup | Down Home Country Coding With Scott Selikoff and Jeanne Boyarsky

Leave a Reply

Your email address will not be published. Required fields are marked *