Attachment 599010 Details for Bug 832124 – Patch to embed id (associated with login) in all forms to combat CSRF

[patch] Patch to embed id (associated with login) in all forms to combat CSRF

CSRF_modified.patch (text/plain), 7.12 KB, created by Trevor McKay on 2012-07-18 22:01:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Trevor McKay

Created: 2012-07-18 22:01:51 UTC

Size: 7.12 KB

patch

obsolete

>diff -rupN ../trunk_unchanged/cumin/python/cumin/account/widgets.py ./cumin/python/cumin/account/widgets.py
>--- ../trunk_unchanged/cumin/python/cumin/account/widgets.py	2012-07-18 10:36:30.322590277 -0400
>+++ ./cumin/python/cumin/account/widgets.py	2012-07-18 17:50:36.751622508 -0400
>@@ -84,6 +84,7 @@ class LoginPage(HtmlPage):
>         if self.logout.get(session):
>             try:
>                 del session.client_session.attributes["login_session"]
>+                session.client_session.reset_csrf()
>             except KeyError:
>                 pass
> 
>diff -rupN ../trunk_unchanged/cumin/python/cumin/widgets.py ./cumin/python/cumin/widgets.py
>--- ../trunk_unchanged/cumin/python/cumin/widgets.py	2012-07-18 10:36:30.321590264 -0400
>+++ ./cumin/python/cumin/widgets.py	2012-07-18 17:52:42.444141096 -0400
>@@ -1372,10 +1372,10 @@ class CuminPage(HtmlPage):
>         return False
> 
>     def redirect_on_exception(self, session):
>-        # If we have an exception from a missing object, redirect to the
>-        # main page with a notice instead of using the not_found_tmpl below.
>-        # Test for presence on the main page already to avoid any possibility
>-        # of an infinite redirect loop.
>+        # If we have certain exceptions, redirect to the main page with a
>+        # notice instead of using the standard templates. Test for presence
>+        # on the main page already to avoid any possibility of an infinite
>+        # redirect loop.
>         cls, value, traceback = sys.exc_info()
>         if self.app.authorizator.is_enforcing():
>             mainpage = self.app.authorizator.find_mainpage(session)
>@@ -1384,11 +1384,16 @@ class CuminPage(HtmlPage):
>         else:
>             mainpage = "/index.html"
> 
>-        if cls is RosemaryNotFound and \
>-           session.request_environment["REQUEST_URI"] != mainpage:
>-            session.add_notice(Notice(
>-                    "An object being displayed became unavailable"))
>-            return mainpage
>+        if session.request_environment["REQUEST_URI"] != mainpage:
>+            if cls is RosemaryNotFound:
>+                session.add_notice(Notice(
>+                        "An object being displayed became unavailable"))
>+            elif cls is CSRFException:
>+                session.add_notice(Notice("An invalid form was submitted"))
>+            else:
>+                mainpage = None
>+
>+        return mainpage
> 
>     def render_error(self, session):
>         cls, value, traceback = sys.exc_info()
>diff -rupN ../trunk_unchanged/wooly/python/wooly/forms.py ./wooly/python/wooly/forms.py
>--- ../trunk_unchanged/wooly/python/wooly/forms.py	2012-07-18 10:36:17.928425238 -0400
>+++ ./wooly/python/wooly/forms.py	2012-07-18 17:50:36.748622474 -0400
>@@ -18,7 +18,7 @@ class Form(Widget):
> 
>     def validate(self, session):
>         log.debug("Validating %s", self)
>-
>+        
>     def render_hidden_inputs(self, session, *args):
>         writer = Writer()
> 
>@@ -45,6 +45,12 @@ class Form(Widget):
>                     svalue = param.marshal(value)
>                     self.write_hidden_input(key, svalue, writer)
> 
>+        # Generate this explicitly.  This value is never set in the
>+        # session except during processing of a POST
>+        self.write_hidden_input(session.csrf_tag_name, 
>+                                session.client_session.get_csrf(), 
>+                                writer)
>+
>         return writer.to_string()
> 
>     def write_hidden_input(self, name, value, writer):
>diff -rupN ../trunk_unchanged/wooly/python/wooly/__init__.py ./wooly/python/wooly/__init__.py
>--- ../trunk_unchanged/wooly/python/wooly/__init__.py	2012-07-18 10:36:17.929425251 -0400
>+++ ./wooly/python/wooly/__init__.py	2012-07-18 17:50:36.749622485 -0400
>@@ -6,6 +6,9 @@ from traceback import print_exception
> log = logging.getLogger("wooly")
> strings = StringCatalog(__file__)
> 
>+class CSRFException(Exception):
>+    pass
>+
> # XXX make this more definitely WidgetAttribute
> class Attribute(object):
>     def __init__(self, app, name):
>@@ -475,6 +478,7 @@ class Page(Frame):
>     def init_parameter(self, param):
>         assert not self.sealed
>         assert isinstance(param, Parameter)
>+        assert param.path != Session.csrf_tag_name
> 
>         self.page_parameters.append(param)
>         self.page_parameters_by_path[param.path] = param
>@@ -696,6 +700,8 @@ class Application(object):
> class Session(object):
>     http_date_gmt = "%a, %d %b %Y %H:%M:%S GMT"
> 
>+    csrf_tag_name = "csrf"
>+
>     def __init__(self, page):
>         assert isinstance(page, Page)
> 
>@@ -713,6 +719,16 @@ class Session(object):
> 
>         self.messages = list()
> 
>+        # This will be the value of the csrf tag if
>+        # it was passed in the request
>+        self.csrf_value = None
>+        self.post = False
>+
>+    def check_csrf(self):
>+        if self.client_session.get_csrf() != self.csrf_value:
>+            log.info("Possible CSRF attempt using %s" % self)
>+            raise CSRFException("Possible CSRF attempt")
>+    
>     def branch(self):
>         session = Session(self.page)
>         session.trunk = self
>@@ -792,7 +808,7 @@ class Session(object):
>                     vars.append("%s=%s" % (key, svalue))
> 
>         return separator.join(vars)
>-
>+            
>     def gather_values(self):
>         if self.trunk is None:
>             return self.values_by_path
>@@ -844,6 +860,8 @@ class Session(object):
> 
>             if param:
>                 param.add(self, param.unmarshal(value), key)
>+            elif key == self.csrf_tag_name:
>+                self.csrf_value = value
> 
>     def marshal_cookies(self):
>         """
>diff -rupN ../trunk_unchanged/wooly/python/wooly/server.py ./wooly/python/wooly/server.py
>--- ../trunk_unchanged/wooly/python/wooly/server.py	2012-07-18 10:36:30.320590250 -0400
>+++ ./wooly/python/wooly/server.py	2012-07-18 17:50:36.750622496 -0400
>@@ -121,6 +121,8 @@ class WebServer(object):
>         if last_modified is None or last_requested is None \
>                 or last_modified > last_requested:
>             try:
>+                if session.post:
>+                    session.check_csrf()
>                 content = page.service(session)
>                 status = "200 OK"
>             except PageRedirect:
>@@ -169,7 +171,8 @@ class WebServer(object):
>     def adapt_request_to_session(self, env, session):
>         session.unmarshal_url_vars(env["QUERY_STRING"])
> 
>-        if env["REQUEST_METHOD"] == "POST":
>+        session.post = env["REQUEST_METHOD"] == "POST"
>+        if session.post:
>             content_type = env["CONTENT_TYPE"]
> 
>             if content_type == "application/x-www-form-urlencoded":
>@@ -280,6 +283,8 @@ class WebServerDispatchThread(Thread):
> class ClientSession(object):
>     def __init__(self):
>         self.id = unique_id()
>+        self.reset_csrf()
>+
>         self.created = datetime.now()
>         self.visited = None
> 
>@@ -288,6 +293,12 @@ class ClientSession(object):
>     def check_owner(self, owner):
>         user = self.attributes["login_session"].user.name
>         return owner == user
>+
>+    def get_csrf(self):
>+        return self.csrf
>+
>+    def reset_csrf(self):
>+        self.csrf = unique_id()
>         
>     def __repr__(self):
>         args = (self.__class__.__name__, self.id, self.created)

Actions: View | Diff

Attachments on bug 832124: 598739 | 598761 | 599010