Merge pull request #2 from gratipay/security-qf35us

fix bugs in CRLF injection protection

aspen/http/baseheaders.py

@@ -9,11 +9,22 @@
 
 
 from aspen.backcompat import CookieError, SimpleCookie
-
 from aspen.http.mapping import CaseInsensitiveMapping
 from aspen.utils import typecheck
 
 
+def _check_for_CRLF(value):
+    """CRLF injection allows an attacker to insert arbitrary headers.
+
+    http://www.acunetix.com/websitesecurity/crlf-injection/
+    https://github.com/gratipay/security-qf35us/issues/1
+
+    """
+    if b'\r' in value or b'\n' in value:
+        from aspen.exceptions import CRLFInjection
+        raise CRLFInjection()
+
+
 class BaseHeaders(CaseInsensitiveMapping):
     """Represent the headers in an HTTP Request or Response message.
        http://stackoverflow.com/questions/5423223/how-to-send-non-english-unicode-string-using-http-header
@@ -54,16 +65,13 @@ def genheaders():
 
 
     def __setitem__(self, name, value):
-        """Extend to protect against CRLF injection:
-
-        http://www.acunetix.com/websitesecurity/crlf-injection/
-
-        """
-        if b'\n' in value:
-            from aspen.exceptions import CRLFInjection
-            raise CRLFInjection()
+        _check_for_CRLF(value)
         super(BaseHeaders, self).__setitem__(name, value)
 
+    def add(self, name, value):
+        _check_for_CRLF(value)
+        super(BaseHeaders, self).add(name, value)
+
 
     def raw(self):
         """Return the headers as a string, formatted for an HTTP message.

tests/test_mappings.py

@@ -7,12 +7,9 @@
 from pytest import raises
 
 from aspen import Response
-
+from aspen.exceptions import CRLFInjection
 from aspen.http.mapping import Mapping, CaseInsensitiveMapping
-
 from aspen.http.baseheaders import BaseHeaders
-from aspen.http.request import Querystring
-
 
 
 def test_mapping_subscript_assignment_clobbers():
@@ -215,3 +212,19 @@ def test_case_insensitive_mapping_ones_is_case_insensitive():
 def test_headers_can_be_raw_when_non_ascii():
     headers = BaseHeaders(b'Foo: bëar\r\nOh: Yeah!')
     assert headers.raw == b'Foo: bëar\r\nOh: Yeah!'
+
+def test_headers_reject_CR_injection():
+    with raises(CRLFInjection):
+        BaseHeaders(b'')[b'foo'] = b'\rbar'
+
+def test_headers_reject_LF_injection():
+    with raises(CRLFInjection):
+        BaseHeaders(b'')[b'foo'] = b'\nbar'
+
+def test_headers_reject_CR_injection_from_add():
+    with raises(CRLFInjection):
+        BaseHeaders(b'').add(b'foo', b'\rbar')
+
+def test_headers_reject_LF_injection_from_add():
+    with raises(CRLFInjection):
+        BaseHeaders(b'').add(b'foo', b'\nbar')