From bdd77f9150fde59d71fdc837ff4cd2cbf0d62896 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 14 Mar 2018 10:34:45 +0000
Subject: [PATCH 1/2] Escape special characters in search by recipient
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SQLAlchemy handles escaping anything that could allow a SQL injection
attack. But it doesn’t escape the characters used for wildcard
searching. This is the reason we’re able to do `.like('%example%')`
at all.

But we shouldn’t be letting our users search with wildcard characters,
so we need to escape them. Which is what this commit does.
---
 app/dao/notifications_dao.py                  |  6 +++
 .../notification_dao/test_notification_dao.py | 39 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py
index 3e7e4ff12..a7a8be166 100644
--- a/app/dao/notifications_dao.py
+++ b/app/dao/notifications_dao.py
@@ -455,6 +455,12 @@ def dao_get_notifications_by_to_field(service_id, search_term, notification_type
     else:
         raise InvalidRequest("Only email and SMS can use search by recipient", 400)
 
+    for special_character in {'_', '%', '/'}:
+        normalised = normalised.replace(
+            special_character,
+            '\\{}'.format(special_character)
+        )
+
     filters = [
         Notification.service_id == service_id,
         Notification.normalised_to.like("%{}%".format(normalised)),
diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py
index db9d999a3..0d5756897 100644
--- a/tests/app/dao/notification_dao/test_notification_dao.py
+++ b/tests/app/dao/notification_dao/test_notification_dao.py
@@ -1769,6 +1769,45 @@ def test_dao_get_notifications_by_to_field_matches_partial_emails(sample_email_t
     assert notification_2.id not in notification_ids
 
 
+@pytest.mark.parametrize('search_term, expected_result_count', [
+    ('foobar', 1),
+    ('foo', 2),
+    ('bar', 2),
+    ('foo%', 1),
+    ('%%bar', 1),
+    ('%_', 1),
+    ('%', 2),
+    ('_', 1),
+    ('/', 1),
+    ('%foo', 0),
+    ('%_%', 0),
+    ('example.com', 4),
+])
+def test_dao_get_notifications_by_to_field_escapes(
+    sample_email_template,
+    search_term,
+    expected_result_count,
+):
+
+    for email_address in {
+        'foo%_@example.com',
+        '%%bar@example.com',
+        'foobar@example.com',
+        '/@example.com',
+    }:
+        create_notification(
+            template=sample_email_template,
+            to_field=email_address,
+            normalised_to=email_address,
+        )
+
+    assert len(dao_get_notifications_by_to_field(
+        sample_email_template.service_id,
+        search_term,
+        notification_type='email',
+    )) == expected_result_count
+
+
 @pytest.mark.parametrize('search_term', [
     '001',
     '100',

From 9103ca59751ce216bac7a433dda873e942609d95 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 14 Mar 2018 11:29:19 +0000
Subject: [PATCH 2/2] Also escape backslashes in search terms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

So that our users can’t accidentally escape characters themselves.
---
 app/dao/notifications_dao.py                            | 4 ++--
 tests/app/dao/notification_dao/test_notification_dao.py | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py
index a7a8be166..d55d546db 100644
--- a/app/dao/notifications_dao.py
+++ b/app/dao/notifications_dao.py
@@ -455,10 +455,10 @@ def dao_get_notifications_by_to_field(service_id, search_term, notification_type
     else:
         raise InvalidRequest("Only email and SMS can use search by recipient", 400)
 
-    for special_character in {'_', '%', '/'}:
+    for special_character in ('\\', '_', '%', '/'):
         normalised = normalised.replace(
             special_character,
-            '\\{}'.format(special_character)
+            '\{}'.format(special_character)
         )
 
     filters = [
diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py
index 0d5756897..3ebbb2a0d 100644
--- a/tests/app/dao/notification_dao/test_notification_dao.py
+++ b/tests/app/dao/notification_dao/test_notification_dao.py
@@ -1779,9 +1779,11 @@ def test_dao_get_notifications_by_to_field_matches_partial_emails(sample_email_t
     ('%', 2),
     ('_', 1),
     ('/', 1),
+    ('\\', 1),
+    ('baz\\baz', 1),
     ('%foo', 0),
     ('%_%', 0),
-    ('example.com', 4),
+    ('example.com', 5),
 ])
 def test_dao_get_notifications_by_to_field_escapes(
     sample_email_template,
@@ -1794,6 +1796,7 @@ def test_dao_get_notifications_by_to_field_escapes(
         '%%bar@example.com',
         'foobar@example.com',
         '/@example.com',
+        'baz\\baz@example.com',
     }:
         create_notification(
             template=sample_email_template,