CVE-2024-29888: Insufficient Information (NVD-CWE-noinfo)
value: Saleor is an e-commerce platform that serves high-volume companies. When using `Pickup: Local stock only` click-and-collect as a delivery method in specific conditions the customer could overwrite the warehouse address with its own, which exposes its address as click-and-collect address. This issue has been patched in versions: `3.14.61`, `3.15.37`, `3.16.34`, `3.17.32`, `3.18.28`, `3.19.15`.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-27T19:15Z
Repository: https://github.com/saleor/saleor
Language: Python
Files changed: saleor/checkout/complete_checkout.py, saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_payment.py, saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_transactions.py, saleor/order/migrations/0172_update_order_cc_addresses.py, saleor/order/migrations/0176_merge_20240325_1315.py, saleor/order/migrations/0177_merge_20240325_1329.py, saleor/order/migrations/0180_merge_20240325_1333.py, saleor/order/migrations/0182_merge_20240325_1338.py, saleor/order/migrations/0184_merge_0182_merge_20240325_1338_0183_order_tax_error.py, saleor/order/migrations/tasks/saleor3_19.py
Commit date: 2024-03-27 13:56:53 +0100
Commit message: Fix relation between order and click and collect address (#15697)

* Create a copy of collection point address during checkout completion

* Add migration for creating order shipping addresses
Diff: diff --git a/saleor/checkout/complete_checkout.py b/saleor/checkout/complete_checkout.py
index 0cdd879529..a974813af6 100644
--- a/saleor/checkout/complete_checkout.py
+++ b/saleor/checkout/complete_checkout.py
@@ -143,10 +143,13 @@ def _process_shipping_data_for_order(
             checkout_info.user, shipping_address, AddressType.SHIPPING, manager=manager
         )
         if checkout_info.user.addresses.filter(pk=shipping_address.pk).exists():
             shipping_address = shipping_address.get_copy()
 
+    if shipping_address and delivery_method_info.warehouse_pk:
+        shipping_address = shipping_address.get_copy()
+
     shipping_method = delivery_method_info.delivery_method
     tax_class = getattr(shipping_method, "tax_class", None)
 
     result: dict[str, Any] = {
         "shipping_address": shipping_address,
diff --git a/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_payment.py b/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_payment.py
index 21b6804742..901904b478 100644
--- a/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_payment.py
+++ b/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_payment.py
@@ -3103,11 +3103,11 @@ def test_complete_checkout_for_local_click_and_collect(
 ):
     # given
     order_count = Order.objects.count()
     checkout = checkout_with_item_for_cc
     checkout.collection_point = warehouse_for_cc
-    checkout.shipping_address = None
+    checkout.shipping_address = warehouse_for_cc.address
     checkout.save(update_fields=["collection_point", "shipping_address"])
 
     variables = {
         "id": to_global_id_or_none(checkout),
         "redirectUrl": "https://www.example.com",
@@ -3145,11 +3145,12 @@ def test_complete_checkout_for_local_click_and_collect(
 
     order = Order.objects.first()
 
     assert order.collection_point == warehouse_for_cc
     assert order.shipping_method is None
-    assert order.shipping_address == warehouse_for_cc.address
+    assert order.shipping_address
+    assert order.shipping_address.id != warehouse_for_cc.address.id
     assert order.shipping_price == zero_taxed_money(payment.currency)
     assert order.lines.count() == 1
 
     # ensure the allocation is made on the correct warehouse
     assert order.lines.first().allocations.first().stock.warehouse == warehouse_for_cc
@@ -3213,11 +3214,12 @@ def test_complete_checkout_for_global_click_and_collect(
 
     order = Order.objects.latest("created_at")
 
     assert order.collection_point == warehouse_for_cc
     assert order.shipping_method is None
-    assert order.shipping_address == warehouse_for_cc.address
+    assert order.shipping_address
+    assert order.shipping_address.id != warehouse_for_cc.address.id
     assert order.shipping_price == zero_taxed_money(payment.currency)
     assert order.lines.count() == 1
 
     # ensure the allocation is made on the correct warehouse
     assert order.lines.first().allocations.first().stock.warehouse == warehouse_for_cc
diff --git a/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_transactions.py b/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_transactions.py
index a7c39822c0..c553436e27 100644
--- a/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_transactions.py
+++ b/saleor/graphql/checkout/tests/mutations/test_checkout_complete_with_transactions.py
@@ -2771,11 +2771,12 @@ def test_complete_checkout_for_local_click_and_collect(
 
     order = Order.objects.first()
 
     assert order.collection_point == warehouse_for_cc
     assert order.shipping_method is None
-    assert order.shipping_address == warehouse_for_cc.address
+    assert order.shipping_address
+    assert order.shipping_address.id != warehouse_for_cc.address.id
     assert order.shipping_price == zero_taxed_money(order.channel.currency_code)
     assert order.lines.count() == 1
 
     # ensure the allocation is made on the correct warehouse
     assert order.lines.first().allocations.first().stock.warehouse == warehouse_for_cc
@@ -2831,11 +2832,12 @@ def test_complete_checkout_for_global_click_and_collect(
 
     order = Order.objects.latest("created_at")
 
     assert order.collection_point == warehouse_for_cc
     assert order.shipping_method is None
-    assert order.shipping_address == warehouse_for_cc.address
+    assert order.shipping_address
+    assert order.shipping_address.id != warehouse_for_cc.address.id
     assert order.shipping_price == zero_taxed_money(order.channel.currency_code)
     assert order.lines.count() == 1
 
     # ensure the allocation is made on the correct warehouse
     assert order.lines.first().allocations.first().stock.warehouse == warehouse_for_cc
diff --git a/saleor/order/migrations/0172_update_order_cc_addresses.py b/saleor/order/migrations/0172_update_order_cc_addresses.py
new file mode 100644
index 0000000000..773ebd9bbb
--- /dev/null
+++ b/saleor/order/migrations/0172_update_order_cc_addresses.py
@@ -0,0 +1,39 @@
+from django.db import migrations
+from django.db.models import Exists, OuterRef
+from django.forms.models import model_to_dict
+
+from .tasks.saleor3_19 import update_order_addresses_task
+
+# The batch of size 250 takes ~0.5 second and consumes ~20MB memory at peak
+ADDRESS_UPDATE_BATCH_SIZE = 250
+
+
+def update_order_addresses(apps, schema_editor):
+    Order = apps.get_model("order", "Order")
+    Warehouse = apps.get_model("warehouse", "Warehouse")
+    Address = apps.get_model("account", "Address")
+    qs = Order.objects.filter(
+        Exists(Warehouse.objects.filter(address_id=OuterRef("shipping_address_id"))),
+    )
+    order_ids = qs.values_list("pk", flat=True)[:ADDRESS_UPDATE_BATCH_SIZE]
+    addresses = []
+    if order_ids:
+        orders = Order.objects.filter(id__in=order_ids)
+        for order in orders:
+            if cc_address := order.shipping_address:
+                order_address = Address(**model_to_dict(cc_address, exclude=["id"]))
+                order.shipping_address = order_address
+                addresses.append(order_address)
+        Address.objects.bulk_create(addresses, ignore_conflicts=True)
+        Order.objects.bulk_update(orders, ["shipping_address"])
+        update_order_addresses_task.delay()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0171_order_order_user_email_user_id_idx"),
+    ]
+
+    operations = [
+        migrations.RunPython(update_order_addresses, migrations.RunPython.noop),
+    ]
diff --git a/saleor/order/migrations/0176_merge_20240325_1315.py b/saleor/order/migrations/0176_merge_20240325_1315.py
new file mode 100644
index 0000000000..7bb6b94801
--- /dev/null
+++ b/saleor/order/migrations/0176_merge_20240325_1315.py
@@ -0,0 +1,12 @@
+# Generated by Django 3.2.22 on 2024-03-25 13:15
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0172_update_order_cc_addresses"),
+        ("order", "0175_merge_20231122_1040"),
+    ]
+
+    operations = []
diff --git a/saleor/order/migrations/0177_merge_20240325_1329.py b/saleor/order/migrations/0177_merge_20240325_1329.py
new file mode 100644
index 0000000000..2fd4932977
--- /dev/null
+++ b/saleor/order/migrations/0177_merge_20240325_1329.py
@@ -0,0 +1,12 @@
+# Generated by Django 3.2.22 on 2024-03-25 13:29
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0176_merge_20231122_1346"),
+        ("order", "0176_merge_20240325_1315"),
+    ]
+
+    operations = []
diff --git a/saleor/order/migrations/0180_merge_20240325_1333.py b/saleor/order/migrations/0180_merge_20240325_1333.py
new file mode 100644
index 0000000000..675bc15cab
--- /dev/null
+++ b/saleor/order/migrations/0180_merge_20240325_1333.py
@@ -0,0 +1,12 @@
+# Generated by Django 3.2.22 on 2024-03-25 13:33
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0177_merge_20240325_1329"),
+        ("order", "0179_merge_20231122_1348"),
+    ]
+
+    operations = []
diff --git a/saleor/order/migrations/0182_merge_20240325_1338.py b/saleor/order/migrations/0182_merge_20240325_1338.py
new file mode 100644
index 0000000000..d11d3a3d82
--- /dev/null
+++ b/saleor/order/migrations/0182_merge_20240325_1338.py
@@ -0,0 +1,12 @@
+# Generated by Django 3.2.22 on 2024-03-25 13:38
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0180_merge_20240325_1333"),
+        ("order", "0181_order_subtotal_as_a_field"),
+    ]
+
+    operations = []
diff --git a/saleor/order/migrations/0184_merge_0182_merge_20240325_1338_0183_order_tax_error.py b/saleor/order/migrations/0184_merge_0182_merge_20240325_1338_0183_order_tax_error.py
new file mode 100644
index 0000000000..0b8a3c2190
--- /dev/null
+++ b/saleor/order/migrations/0184_merge_0182_merge_20240325_1338_0183_order_tax_error.py
@@ -0,0 +1,12 @@
+# Generated by Django 3.2.22 on 2024-03-25 13:42
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("order", "0182_merge_20240325_1338"),
+        ("order", "0183_order_tax_error"),
+    ]
+
+    operations = []
diff --git a/saleor/order/migrations/tasks/saleor3_19.py b/saleor/order/migrations/tasks/saleor3_19.py
new file mode 100644
index 0000000000..ddc468009b
--- /dev/null
+++ b/saleor/order/migrations/tasks/saleor3_19.py
@@ -0,0 +1,29 @@
+from django.db.models import Exists, OuterRef
+from django.forms.models import model_to_dict
+
+from ....account.models import Address
+from ....celeryconf import app
+from ....warehouse.models import Warehouse
+from ...models import Order
+
+# The batch of size 250 takes ~0.5 second and consumes ~20MB memory at peak
+ADDRESS_UPDATE_BATCH_SIZE = 250
+
+
+@app.task
+def update_order_addresses_task():
+    qs = Order.objects.filter(
+        Exists(Warehouse.objects.filter(address_id=OuterRef("shipping_address_id"))),
+    )
+    order_ids = qs.values_list("pk", flat=True)[:ADDRESS_UPDATE_BATCH_SIZE]
+    addresses = []
+    if order_ids:
+        orders = Order.objects.filter(id__in=order_ids)
+        for order in orders:
+            if cc_address := order.shipping_address:
+                order_address = Address(**model_to_dict(cc_address, exclude=["id"]))
+                order.shipping_address = order_address
+                addresses.append(order_address)
+        Address.objects.bulk_create(addresses, ignore_conflicts=True)
+        Order.objects.bulk_update(orders, ["shipping_address"])
+        update_order_addresses_task.delay()

Vulnerable code:     checkout.shipping_address = None
    assert order.shipping_address == warehouse_for_cc.address
    assert order.shipping_address == warehouse_for_cc.address
    assert order.shipping_address == warehouse_for_cc.address
    assert order.shipping_address == warehouse_for_cc.address
Fixed code:     if shipping_address and delivery_method_info.warehouse_pk:
        shipping_address = shipping_address.get_copy()

    checkout.shipping_address = warehouse_for_cc.address
    assert order.shipping_address
    assert order.shipping_address.id != warehouse_for_cc.address.id
    assert order.shipping_address
    assert order.shipping_address.id != warehouse_for_cc.address.id
    assert order.shipping_address
    assert order.shipping_address.id != warehouse_for_cc.address.id
    assert order.shipping_address
    assert order.shipping_address.id != warehouse_for_cc.address.id
from django.db import migrations
from django.db.models import Exists, OuterRef
from django.forms.models import model_to_dict

from .tasks.saleor3_19 import update_order_addresses_task

# The batch of size 250 takes ~0.5 second and consumes ~20MB memory at peak
ADDRESS_UPDATE_BATCH_SIZE = 250


def update_order_addresses(apps, schema_editor):
    Order = apps.get_model("order", "Order")
    Warehouse = apps.get_model("warehouse", "Warehouse")
    Address = apps.get_model("account", "Address")
    qs = Order.objects.filter(
        Exists(Warehouse.objects.filter(address_id=OuterRef("shipping_address_id"))),
    )
    order_ids = qs.values_list("pk", flat=True)[:ADDRESS_UPDATE_BATCH_SIZE]
    addresses = []
    if order_ids:
        orders = Order.objects.filter(id__in=order_ids)
        for order in orders:
            if cc_address := order.shipping_address:
                order_address = Address(**model_to_dict(cc_address, exclude=["id"]))
                order.shipping_address = order_address
                addresses.append(order_address)
        Address.objects.bulk_create(addresses, ignore_conflicts=True)
        Order.objects.bulk_update(orders, ["shipping_address"])
        update_order_addresses_task.delay()


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0171_order_order_user_email_user_id_idx"),
    ]

    operations = [
        migrations.RunPython(update_order_addresses, migrations.RunPython.noop),
    ]
# Generated by Django 3.2.22 on 2024-03-25 13:15

from django.db import migrations


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0172_update_order_cc_addresses"),
        ("order", "0175_merge_20231122_1040"),
    ]

    operations = []
# Generated by Django 3.2.22 on 2024-03-25 13:29

from django.db import migrations


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0176_merge_20231122_1346"),
        ("order", "0176_merge_20240325_1315"),
    ]

    operations = []
# Generated by Django 3.2.22 on 2024-03-25 13:33

from django.db import migrations


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0177_merge_20240325_1329"),
        ("order", "0179_merge_20231122_1348"),
    ]

    operations = []
# Generated by Django 3.2.22 on 2024-03-25 13:38

from django.db import migrations


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0180_merge_20240325_1333"),
        ("order", "0181_order_subtotal_as_a_field"),
    ]

    operations = []
# Generated by Django 3.2.22 on 2024-03-25 13:42

from django.db import migrations


class Migration(migrations.Migration):
    dependencies = [
        ("order", "0182_merge_20240325_1338"),
        ("order", "0183_order_tax_error"),
    ]

    operations = []
from django.db.models import Exists, OuterRef
from django.forms.models import model_to_dict

from ....account.models import Address
from ....celeryconf import app
from ....warehouse.models import Warehouse
from ...models import Order

# The batch of size 250 takes ~0.5 second and consumes ~20MB memory at peak
ADDRESS_UPDATE_BATCH_SIZE = 250


@app.task
def update_order_addresses_task():
    qs = Order.objects.filter(
        Exists(Warehouse.objects.filter(address_id=OuterRef("shipping_address_id"))),
    )
    order_ids = qs.values_list("pk", flat=True)[:ADDRESS_UPDATE_BATCH_SIZE]
    addresses = []
    if order_ids:
        orders = Order.objects.filter(id__in=order_ids)
        for order in orders:
            if cc_address := order.shipping_address:
                order_address = Address(**model_to_dict(cc_address, exclude=["id"]))
                order.shipping_address = order_address
                addresses.append(order_address)
        Address.objects.bulk_create(addresses, ignore_conflicts=True)
        Order.objects.bulk_update(orders, ["shipping_address"])
        update_order_addresses_task.delay()