Florian Weimer found that Cumin did not protect against CSRF (Cross-Site Request Forgery) attacks. If an authenticated user logged into a Cumin web site visited a specially-crafted web page, it could lead to the unauthorized command execution in the Cumin site with the privileges of the logged-in user.
Created attachment 598739 [details] Patch to embed id (associated with login) in all forms to combat CSRF
This patch should add CSRF protection. A unique id (generated from the same source that generates session ids) is embedded in the rendering of every descendent of the Form class, unconditionally. The unique id is stored in the login record for the session and compared against the value in a POST request. The name of the csrf token is based on the name of the particular form (using the standard parameter.path naming system). The only form(s) where the token will be unset are login forms. I suppose a CSRF attack could be used to log a user in as another legitimate user, but would this be dangerous? The patch could be extended to initiliaze a CSRF token to use for login forms, with a reset of the id after login. Giving it a try...
Created attachment 598761 [details] Patch to embed id (associated with login) in all forms to combat CSRF Moved the csrf tag into the client_session but outside of the login record. The id is set on creation and reset on login and logout. Now all forms will have an embedded id, regardless of login state. Tested with crafted login POST. Note, this is starting to collide with the patch for session fixation. The ultimate form will be a little different (messing with the csrf tag value at login will be part of the routine that resets the session id).
Note on processing. The CSRF checks are done in the processing pass on descendents of the form class. Processing is done in depth first order over the widget hierarchy, so a form's ancestors in the hierarchy may be processed before the CSRF check fires (procesing may not be *strictly* depth first, that is a widget may take some action before calling do_process() on its children). This means that for some forms, there is a small window where processing may happen in parent widgets before an exception is raised and processing stops in a CSRF case. Thinking about a streamlined way to move CSRF checks to the beginning of processing.... One possibility: the csrf tags are named for the widget (path) to which they pertain. So, scanning the POST data for csrf values gives us a list of paths to all form widgets touched in the POST (probably only one). Pages hold lists of widgets by path, therefore we ought to be able to call methods on those widgets early (or be less general and just check the CSRF values against the client session, without the possibility of an overload).
Created attachment 599010 [details] Patch to embed id (associated with login) in all forms to combat CSRF Greatly simplified, and early checks solved. (can you say "blinders" :) ) There is a single global well known name for the csrf tag value in forms, and it is generated and checked without any reliance on the widget and parameter classes or the widget processing methods.
Acknowledgements: This issue was discovered by Florian Weimer of the Red Hat Product Security Team.
This issue has been addressed in following products: MRG for RHEL-5 v. 2 Via RHSA-2012:1278 https://rhn.redhat.com/errata/RHSA-2012-1278.html
This issue has been addressed in following products: MRG for RHEL-6 v.2 Via RHSA-2012:1281 https://rhn.redhat.com/errata/RHSA-2012-1281.html
Created cumin tracking bugs for this issue Affects: fedora-all [bug 858868]