From 1561492ff7ab656821214ade691f5f8c7e3a1931 Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Mon, 1 Dec 2025 23:23:42 +0530 Subject: [PATCH 1/7] Fix double refund race condition vulnerability --- app/eventyay/api/views/order.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/eventyay/api/views/order.py b/app/eventyay/api/views/order.py index ed2491d49e..6d38399d77 100644 --- a/app/eventyay/api/views/order.py +++ b/app/eventyay/api/views/order.py @@ -1247,8 +1247,11 @@ def confirm(self, request, **kwargs): return self.retrieve(request, [], **kwargs) @action(detail=True, methods=['POST']) + @transaction.atomic def refund(self, request, **kwargs): - payment = self.get_object() + # Acquire row-level lock on payment to prevent concurrent refund race conditions + payment = OrderPayment.objects.select_for_update().get(pk=self.get_object().pk) + amount = serializers.DecimalField(max_digits=10, decimal_places=2).to_internal_value( request.data.get('amount', str(payment.amount)) ) @@ -1265,6 +1268,7 @@ def refund(self, request, **kwargs): full_refund_possible = payment.payment_provider.payment_refund_supported(payment) partial_refund_possible = payment.payment_provider.payment_partial_refund_supported(payment) + # Calculate available amount under lock - prevents TOCTOU race condition available_amount = payment.amount - payment.refunded_amount if amount <= 0: From 4ea2aaf34fbb0cb8af642f0ad209b9e62d751fd3 Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Mon, 1 Dec 2025 23:37:52 +0530 Subject: [PATCH 2/7] optimize refund locking to single query. --- app/eventyay/api/views/order.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/eventyay/api/views/order.py b/app/eventyay/api/views/order.py index 6d38399d77..cde4598ff8 100644 --- a/app/eventyay/api/views/order.py +++ b/app/eventyay/api/views/order.py @@ -1249,8 +1249,13 @@ def confirm(self, request, **kwargs): @action(detail=True, methods=['POST']) @transaction.atomic def refund(self, request, **kwargs): - # Acquire row-level lock on payment to prevent concurrent refund race conditions - payment = OrderPayment.objects.select_for_update().get(pk=self.get_object().pk) + # Acquire row-level lock on payment to prevent concurrent refund race conditions. + # We reuse DRF's filtering & permission logic while issuing a single SELECT ... FOR UPDATE. + queryset = self.filter_queryset(self.get_queryset().select_for_update()) + + lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field + lookup_value = self.kwargs[lookup_url_kwarg] + payment = get_object_or_404(queryset, **{self.lookup_field: lookup_value}) amount = serializers.DecimalField(max_digits=10, decimal_places=2).to_internal_value( request.data.get('amount', str(payment.amount)) From d40eacafa346ac8d9bddafdd5cc21cc4e053fa4e Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Tue, 9 Dec 2025 11:03:26 +0530 Subject: [PATCH 3/7] Update app/eventyay/api/views/order.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/eventyay/api/views/order.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/eventyay/api/views/order.py b/app/eventyay/api/views/order.py index cde4598ff8..e6e75bdacc 100644 --- a/app/eventyay/api/views/order.py +++ b/app/eventyay/api/views/order.py @@ -1256,7 +1256,6 @@ def refund(self, request, **kwargs): lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field lookup_value = self.kwargs[lookup_url_kwarg] payment = get_object_or_404(queryset, **{self.lookup_field: lookup_value}) - amount = serializers.DecimalField(max_digits=10, decimal_places=2).to_internal_value( request.data.get('amount', str(payment.amount)) ) From c5278c843b555d9c0998f8173944a27bf69e859b Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Tue, 9 Dec 2025 11:03:46 +0530 Subject: [PATCH 4/7] Update app/eventyay/api/views/order.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/eventyay/api/views/order.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/eventyay/api/views/order.py b/app/eventyay/api/views/order.py index e6e75bdacc..4bf1dab6e2 100644 --- a/app/eventyay/api/views/order.py +++ b/app/eventyay/api/views/order.py @@ -1252,7 +1252,6 @@ def refund(self, request, **kwargs): # Acquire row-level lock on payment to prevent concurrent refund race conditions. # We reuse DRF's filtering & permission logic while issuing a single SELECT ... FOR UPDATE. queryset = self.filter_queryset(self.get_queryset().select_for_update()) - lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field lookup_value = self.kwargs[lookup_url_kwarg] payment = get_object_or_404(queryset, **{self.lookup_field: lookup_value}) From 75e07295481eba0148ac38b2004ab7936af3a266 Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Tue, 9 Dec 2025 11:48:03 +0530 Subject: [PATCH 5/7] added test for race condition --- src/tests/api/test_orders.py | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/tests/api/test_orders.py b/src/tests/api/test_orders.py index e8323e18d6..1263099d0b 100644 --- a/src/tests/api/test_orders.py +++ b/src/tests/api/test_orders.py @@ -3989,6 +3989,67 @@ def test_refund_create_invalid_payment(token_client, organizer, event, order): ) assert resp.status_code == 400 +@pytest.mark.django_db +def test_refund_concurrent_race_condition_prevented(token_client, organizer, event, order): + + import threading + from decimal import Decimal + + with scopes_disabled(): + payment = order.payments.first() + payment.amount = Decimal('100.00') + payment.state = OrderPayment.PAYMENT_STATE_CONFIRMED + payment.save() + + results = {'thread1': None, 'thread2': None, 'errors': []} + + def refund_request(thread_name, amount): + """Make a refund request in a separate thread""" + try: + res = { + 'amount': str(amount), + } + resp = token_client.post( + '/api/v1/organizers/{}/events/{}/orders/{}/payments/{}/refund/'.format( + organizer.slug, event.slug, order.code, payment.local_id + ), + format='json', + data=res, + ) + results[thread_name] = { + 'status': resp.status_code, + 'data': resp.data if hasattr(resp, 'data') else None + } + except Exception as e: + results['errors'].append(str(e)) + + thread1 = threading.Thread(target=refund_request, args=('thread1', Decimal('80.00'))) + thread2 = threading.Thread(target=refund_request, args=('thread2', Decimal('80.00'))) + + thread1.start() + thread2.start() + + thread1.join() + thread2.join() + + assert len(results['errors']) == 0, f"Unexpected errors: {results['errors']}" + + status_codes = [results['thread1']['status'], results['thread2']['status']] + + assert 200 in status_codes, "At least one refund should succeed" + assert 400 in status_codes, "One refund should be rejected due to insufficient available amount" + + with scopes_disabled(): + payment.refresh_from_db() + total_refunded = sum(r.amount for r in payment.refunds.all()) + + assert total_refunded == Decimal('80.00'), f"Expected $80 refunded, got ${total_refunded}" + assert payment.refunds.count() == 1, f"Expected 1 refund, got {payment.refunds.count()}" + + available = payment.amount - total_refunded + assert available == Decimal('20.00'), f"Expected $20 available, got ${available}" + + @pytest.mark.django_db def test_order_delete(token_client, organizer, event, order): From 0970534e79a5ef206fd6e998eac61a39eb535179 Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Tue, 9 Dec 2025 12:01:19 +0530 Subject: [PATCH 6/7] fixed test --- src/tests/api/test_orders.py | 90 +++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/tests/api/test_orders.py b/src/tests/api/test_orders.py index 1263099d0b..0b6f3cb44d 100644 --- a/src/tests/api/test_orders.py +++ b/src/tests/api/test_orders.py @@ -3989,66 +3989,70 @@ def test_refund_create_invalid_payment(token_client, organizer, event, order): ) assert resp.status_code == 400 -@pytest.mark.django_db +@pytest.mark.django_db(transaction=True) def test_refund_concurrent_race_condition_prevented(token_client, organizer, event, order): import threading from decimal import Decimal - + from django.db import connection + from django.test.utils import CaptureQueriesContext + from django.db import connection + from django.test import Client + import json + with scopes_disabled(): payment = order.payments.first() payment.amount = Decimal('100.00') payment.state = OrderPayment.PAYMENT_STATE_CONFIRMED payment.save() - - results = {'thread1': None, 'thread2': None, 'errors': []} - - def refund_request(thread_name, amount): - """Make a refund request in a separate thread""" + + results = [] + errors = [] + + def make_refund_request(amount): try: - res = { - 'amount': str(amount), - } - resp = token_client.post( - '/api/v1/organizers/{}/events/{}/orders/{}/payments/{}/refund/'.format( - organizer.slug, event.slug, order.code, payment.local_id - ), - format='json', - data=res, + client = Client() + client.defaults['HTTP_AUTHORIZATION'] = token_client.defaults.get('HTTP_AUTHORIZATION') + response = client.post( + f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/orders/{order.code}/payments/{payment.local_id}/refund/', + data=json.dumps({'amount': str(amount)}), + content_type='application/json', ) - results[thread_name] = { - 'status': resp.status_code, - 'data': resp.data if hasattr(resp, 'data') else None - } + results.append({'status': response.status_code, 'amount': amount}) except Exception as e: - results['errors'].append(str(e)) - - thread1 = threading.Thread(target=refund_request, args=('thread1', Decimal('80.00'))) - thread2 = threading.Thread(target=refund_request, args=('thread2', Decimal('80.00'))) - + errors.append(str(e)) + finally: + connection.close() + + thread1 = threading.Thread(target=make_refund_request, args=(Decimal('80.00'),)) + thread2 = threading.Thread(target=make_refund_request, args=(Decimal('80.00'),)) + thread1.start() thread2.start() - - thread1.join() - thread2.join() - - assert len(results['errors']) == 0, f"Unexpected errors: {results['errors']}" - - status_codes = [results['thread1']['status'], results['thread2']['status']] - - assert 200 in status_codes, "At least one refund should succeed" - assert 400 in status_codes, "One refund should be rejected due to insufficient available amount" - + + thread1.join(timeout=5) + thread2.join(timeout=5) + + assert len(errors) == 0, f"Unexpected errors: {errors}" + assert len(results) == 2, f"Expected 2 results, got {len(results)}" + + status_codes = sorted([r['status'] for r in results]) + + assert status_codes == [200, 400], \ + f"Expected one success (200) and one failure (400), got {status_codes}" + with scopes_disabled(): payment.refresh_from_db() - total_refunded = sum(r.amount for r in payment.refunds.all()) - - assert total_refunded == Decimal('80.00'), f"Expected $80 refunded, got ${total_refunded}" - assert payment.refunds.count() == 1, f"Expected 1 refund, got {payment.refunds.count()}" - - available = payment.amount - total_refunded - assert available == Decimal('20.00'), f"Expected $20 available, got ${available}" + refunds = list(payment.refunds.all()) + total_refunded = sum(r.amount for r in refunds) + + assert len(refunds) == 1, f"Expected exactly 1 refund object, found {len(refunds)}" + assert total_refunded == Decimal('80.00'), \ + f"Expected total refunded = $80.00, got {total_refunded}" + available = payment.amount - total_refunded + assert available == Decimal('20.00'), \ + f"Expected available = $20.00, got {available}" @pytest.mark.django_db From 1c0e2f3841d77fd251821c651a7adb212455091a Mon Sep 17 00:00:00 2001 From: Arnav Angarkar Date: Wed, 10 Dec 2025 16:37:44 +0530 Subject: [PATCH 7/7] Remove test --- src/tests/api/test_orders.py | 65 ------------------------------------ 1 file changed, 65 deletions(-) diff --git a/src/tests/api/test_orders.py b/src/tests/api/test_orders.py index 0b6f3cb44d..e8323e18d6 100644 --- a/src/tests/api/test_orders.py +++ b/src/tests/api/test_orders.py @@ -3989,71 +3989,6 @@ def test_refund_create_invalid_payment(token_client, organizer, event, order): ) assert resp.status_code == 400 -@pytest.mark.django_db(transaction=True) -def test_refund_concurrent_race_condition_prevented(token_client, organizer, event, order): - - import threading - from decimal import Decimal - from django.db import connection - from django.test.utils import CaptureQueriesContext - from django.db import connection - from django.test import Client - import json - - with scopes_disabled(): - payment = order.payments.first() - payment.amount = Decimal('100.00') - payment.state = OrderPayment.PAYMENT_STATE_CONFIRMED - payment.save() - - results = [] - errors = [] - - def make_refund_request(amount): - try: - client = Client() - client.defaults['HTTP_AUTHORIZATION'] = token_client.defaults.get('HTTP_AUTHORIZATION') - response = client.post( - f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/orders/{order.code}/payments/{payment.local_id}/refund/', - data=json.dumps({'amount': str(amount)}), - content_type='application/json', - ) - results.append({'status': response.status_code, 'amount': amount}) - except Exception as e: - errors.append(str(e)) - finally: - connection.close() - - thread1 = threading.Thread(target=make_refund_request, args=(Decimal('80.00'),)) - thread2 = threading.Thread(target=make_refund_request, args=(Decimal('80.00'),)) - - thread1.start() - thread2.start() - - thread1.join(timeout=5) - thread2.join(timeout=5) - - assert len(errors) == 0, f"Unexpected errors: {errors}" - assert len(results) == 2, f"Expected 2 results, got {len(results)}" - - status_codes = sorted([r['status'] for r in results]) - - assert status_codes == [200, 400], \ - f"Expected one success (200) and one failure (400), got {status_codes}" - - with scopes_disabled(): - payment.refresh_from_db() - refunds = list(payment.refunds.all()) - total_refunded = sum(r.amount for r in refunds) - - assert len(refunds) == 1, f"Expected exactly 1 refund object, found {len(refunds)}" - assert total_refunded == Decimal('80.00'), \ - f"Expected total refunded = $80.00, got {total_refunded}" - - available = payment.amount - total_refunded - assert available == Decimal('20.00'), \ - f"Expected available = $20.00, got {available}" - @pytest.mark.django_db def test_order_delete(token_client, organizer, event, order):