Skip to content

Commit c079983

Browse files
committed
Security: prevent timing attack on WEBHOOK_AUTHORIZATION secret
Anymail's webhook validation was vulnerable to a timing attack. An attacker could have used this to recover your WEBHOOK_AUTHORIZATION shared secret, potentially allowing them to post fabricated or malicious email tracking events to your app. There have not been any reports of attempted exploit in the wild. (The vulnerability was discovered through code review.) Attempts would be visible in http logs as a very large number of 400 responses on Anymail's webhook urls, or in Python error monitoring as a very large number of AnymailWebhookValidationFailure exceptions. If you are using Anymail's webhooks, you should upgrade to this release. In addition, you may want to rotate to a new WEBHOOK_AUTHORIZATION secret ([docs](http://anymail.readthedocs.io/en/stable/tips/securing_webhooks/#use-a-shared-authorization-secret)), particularly if your logs indicate attempted exploit. (cherry picked from commit db586ed)
1 parent 7029298 commit c079983

File tree

1 file changed

+12
-3
lines changed

1 file changed

+12
-3
lines changed

anymail/webhooks/base.py

+12-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import six
55
from django.http import HttpResponse
6+
from django.utils.crypto import constant_time_compare
67
from django.utils.decorators import method_decorator
78
from django.views.decorators.csrf import csrf_exempt
89
from django.views.generic import View
@@ -41,8 +42,13 @@ def __init__(self, **kwargs):
4142
def validate_request(self, request):
4243
"""If configured for webhook basic auth, validate request has correct auth."""
4344
if self.basic_auth:
44-
basic_auth = get_request_basic_auth(request)
45-
if basic_auth is None or basic_auth not in self.basic_auth:
45+
request_auth = get_request_basic_auth(request)
46+
# Use constant_time_compare to avoid timing attack on basic auth. (It's OK that any()
47+
# can terminate early: we're not trying to protect how many auth strings are allowed,
48+
# just the contents of each individual auth string.)
49+
auth_ok = any(constant_time_compare(request_auth, allowed_auth)
50+
for allowed_auth in self.basic_auth)
51+
if not auth_ok:
4652
# noinspection PyUnresolvedReferences
4753
raise AnymailWebhookValidationFailure(
4854
"Missing or invalid basic auth in Anymail %s webhook" % self.esp_name)
@@ -78,8 +84,11 @@ def validate_request(self, request):
7884
*All* definitions of this method in the class chain (including mixins)
7985
will be called. There is no need to chain to the superclass.
8086
(See self.run_validators and collect_all_methods.)
87+
88+
Security note: use django.utils.crypto.constant_time_compare for string
89+
comparisons, to avoid exposing your validation to a timing attack.
8190
"""
82-
# if request.POST['signature'] != expected_signature:
91+
# if not constant_time_compare(request.POST['signature'], expected_signature):
8392
# raise AnymailWebhookValidationFailure("...message...")
8493
# (else just do nothing)
8594
pass

0 commit comments

Comments
 (0)