[tor-commits] [bridgedb/master] Delay and then redirect malicious requests.

[isis at torproject.org]

commit 0f03fb65518353e1409e08def7201ea17fe1318f
Author: Isis Lovecruft <isis at torproject.org>
Date:   Fri Dec 22 01:45:52 2017 +0000

    Delay and then redirect malicious requests.
    
     * FIXES #24701: https://bugs.torproject.org/24701
---
 bridgedb/distributors/https/server.py | 43 +++++++++++++++++++++++++++--------
 bridgedb/test/test_https_server.py    | 11 ++++-----
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/bridgedb/distributors/https/server.py b/bridgedb/distributors/https/server.py
index d4771a6..8c50bc1 100644
--- a/bridgedb/distributors/https/server.py
+++ b/bridgedb/distributors/https/server.py
@@ -37,7 +37,9 @@ import mako.exceptions
 from mako.template import Template
 from mako.lookup import TemplateLookup
 
+from twisted.internet import defer
 from twisted.internet import reactor
+from twisted.internet import task
 from twisted.internet.error import CannotListenError
 from twisted.web import resource
 from twisted.web import static
@@ -138,6 +140,20 @@ def replaceErrorPage(request, error, template_name=None, html=True):
     return rendered
 
 
+def redirectMaliciousRequest(request):
+    '''Redirect the client to a "daring work of art" which "in true
+    post-modern form, […] tends to raise more questions than answers."
+    '''
+    logging.debug("Redirecting %s to a daring work of art..." % getClientIP(request))
+    request.write(redirectTo(base64.b64decode("aHR0cDovLzJnaXJsczFjdXAuY2Ev"), request))
+    request.finish()
+    return request
+
+
+class MaliciousRequest(Exception):
+    """Raised when we received a possibly malicious request."""
+
+
 class CSPResource(resource.Resource):
     """A resource which adds a ``'Content-Security-Policy:'`` header.
 
@@ -411,9 +427,9 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
             challenge = request.args['captcha_challenge_field'][0]
             response = request.args['captcha_response_field'][0]
         except Exception as error:
-            logging.debug(("Client CAPTCHA solution to HTTPS distributor server"
-                           "didn't include correct HTTP arguments: %s" % error))
-            return redirectTo(type(b'')(request.URLPath()), request)
+            raise MaliciousRequest(
+                ("Client CAPTCHA solution to HTTPS distributor server "
+                 "didn't include correct HTTP arguments: %s" % error))
         return (challenge, response)
 
     def checkSolution(self, request):
@@ -477,12 +493,21 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
         self.setCSPHeader(request)
         request.setHeader("Content-Type", "text/html; charset=utf-8")
 
-        if self.checkSolution(request) is True:
-            try:
-                rendered = self.resource.render(request)
-            except Exception as err:
-                rendered = replaceErrorPage(request, err)
-            return rendered
+        try:
+            if self.checkSolution(request) is True:
+                return self.resource.render(request)
+        except ValueError as err:
+            logging.debug(err.message)
+        except MaliciousRequest as err:
+            logging.debug(err.message)
+            # Make them wait a bit, then redirect them to a "daring
+            # work of art" as pennance for their sins.
+            d = task.deferLater(reactor, 1, lambda: request)
+            d.addCallback(redirectMaliciousRequest)
+            return NOT_DONE_YET
+        except Exception as err:
+            logging.debug(err.message)
+            return replaceErrorPage(request, err)
 
         logging.debug("Client failed a CAPTCHA; returning redirect to %s"
                       % request.uri)
diff --git a/bridgedb/test/test_https_server.py b/bridgedb/test/test_https_server.py
index 13ec20e..ba555e8 100644
--- a/bridgedb/test/test_https_server.py
+++ b/bridgedb/test/test_https_server.py
@@ -373,18 +373,17 @@ class GimpCaptchaProtectedResourceTests(unittest.TestCase):
         self.assertEqual(response, expectedResponse)
 
     def test_extractClientSolution_missing_arguments(self):
-        """A solution with missing arguments (the solution field) should
-        return a very agressive redirect to the originally requested,
-        CAPTCHA-protected page.
+        """A solution with missing arguments (the solution/challenge fields)
+        should raise a MaliciousRequest exception.
         """
         expectedChallenge = '23232323232323232323'
 
         self.request.method = b'POST'
         self.request.addArg('captcha_challenge_field', expectedChallenge)
 
-        response = self.captchaResource.extractClientSolution(self.request)
-
-        self.assertIn("click here", response)
+        self.assertRaises(server.MaliciousRequest,
+                          self.captchaResource.extractClientSolution,
+                          self.request)
 
     def test_checkSolution(self):
         """checkSolution() should return False is the solution is invalid."""