4337: Validate init_code (#1999)

- Decode owners using init_code
- Use `init_code` decoded owners for signature validation
- Add helper to decode init_code
- Refactor 4337 serializer

requirements.txt

@@ -33,5 +33,5 @@ psycogreen==1.0.2
 psycopg2==2.9.9
 redis==5.0.3
 requests==2.31.0
-safe-eth-py[django]==6.0.0b23
+safe-eth-py[django]==6.0.0b25
 web3==6.17.0

safe_transaction_service/account_abstraction/helpers.py

@@ -0,0 +1,81 @@
+import dataclasses
+from typing import List
+
+from eth_typing import ChecksumAddress
+
+from gnosis.eth import EthereumClient
+from gnosis.eth.contracts import get_safe_V1_4_1_contract
+from gnosis.eth.utils import fast_to_checksum_address
+from gnosis.safe.proxy_factory import ProxyFactoryV141
+
+
+@dataclasses.dataclass(eq=True, frozen=True)
+class DecodedInitCode:
+    # UserOperation data
+    factory_address: ChecksumAddress
+    factory_data: bytes  # Factory call with function identifier
+    initializer: bytes  # Initializer passed to ProxyFactory
+    # ProxyFactory data
+    singleton: ChecksumAddress
+    salt_nonce: int
+    expected_address: ChecksumAddress  # Expected Safe deployment address
+    # Safe creation data
+    owners: List[ChecksumAddress]
+    threshold: int
+    to: ChecksumAddress
+    data: bytes
+    fallback_handler: ChecksumAddress
+    payment_token: ChecksumAddress
+    payment: int
+    payment_receiver: ChecksumAddress
+
+
+def decode_init_code(
+    init_code: bytes, ethereum_client: EthereumClient
+) -> DecodedInitCode:
+    """
+    Decode data to check for a valid ProxyFactory Safe deployment.
+
+    :param init_code: should be composed of:
+      - 20 first bytes with the address of the factory.
+      - Call data for the ``Factory``. In the case of the Safe:
+        - Call to the ``ProxyFactory``, with the ``initializer``, ``singleton`` and ``saltNonce``
+        - The ``ProxyFactory`` then deploys a ``Safe Proxy`` and calls ``setup`` with all the configuration parameters.
+    :param ethereum_client:
+    :return: Decoded Init Code dataclass
+    :raises ValueError: Problem decoding
+    """
+    factory_address = fast_to_checksum_address(init_code[:20])
+    factory_data = init_code[20:]
+    proxy_factory = ProxyFactoryV141(factory_address, ethereum_client)
+    safe_contract = get_safe_V1_4_1_contract(ethereum_client.w3)
+    _, data = proxy_factory.contract.decode_function_input(factory_data)
+    initializer = data.pop("initializer")
+    _, safe_deployment_data = safe_contract.decode_function_input(initializer)
+
+    singleton = data.pop("_singleton")
+    salt_nonce = data.pop("saltNonce")
+    expected_address = proxy_factory.calculate_proxy_address(
+        singleton, initializer, salt_nonce, chain_specific=False
+    )
+    return DecodedInitCode(
+        factory_address,
+        factory_data,
+        initializer,
+        singleton,
+        salt_nonce,
+        expected_address,
+        *(
+            safe_deployment_data[field]
+            for field in [
+                "_owners",
+                "_threshold",
+                "to",
+                "data",
+                "fallbackHandler",
+                "paymentToken",
+                "payment",
+                "paymentReceiver",
+            ]
+        )
+    )

safe_transaction_service/account_abstraction/serializers.py

@@ -21,6 +21,7 @@
 from safe_transaction_service.utils.ethereum import get_chain_id
 
 from ..utils.serializers import get_safe_owners
+from .helpers import decode_init_code
 from .models import SafeOperation
 from .models import SafeOperation as SafeOperationModel
 from .models import SafeOperationConfirmation
@@ -52,6 +53,14 @@ class SafeOperationSerializer(serializers.Serializer):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.ethereum_client = EthereumClientProvider()
+        self._deployment_owners: List[ChecksumAddress] = []
+
+    def _get_owners(self, safe_address: ChecksumAddress) -> List[ChecksumAddress]:
+        """
+        :param safe_address:
+        :return:  `init_code` decoded owners if Safe is not deployed or current blockchain owners if Safe is deployed
+        """
+        return self._deployment_owners or get_safe_owners(safe_address)
 
     def _validate_signature(
         self,
@@ -60,7 +69,7 @@ def _validate_signature(
         safe_operation_hash_preimage: bytes,
         signature: bytes,
     ) -> List[SafeSignature]:
-        safe_owners = get_safe_owners(safe_address)
+        safe_owners = self._get_owners(safe_address)
         parsed_signatures = SafeSignature.parse_signature(
             signature, safe_operation_hash, safe_operation_hash_preimage
         )
@@ -91,12 +100,35 @@ def validate_init_code(self, init_code: Optional[HexBytes]) -> Optional[HexBytes
         :param init_code:
         :return: `init_code`
         """
+        safe_address = self.context["safe_address"]
+        safe_is_deployed = self.ethereum_client.is_contract(safe_address)
         if init_code:
-            safe_address = self.context["safe_address"]
-            if self.ethereum_client.is_contract(safe_address):
+            if safe_is_deployed:
                 raise ValidationError(
                     "`init_code` must be empty as the contract was already initialized"
                 )
+
+            try:
+                decoded_init_code = decode_init_code(init_code, self.ethereum_client)
+            except ValueError:
+                raise ValidationError("Cannot decode data")
+            if not self.ethereum_client.is_contract(decoded_init_code.factory_address):
+                raise ValidationError(
+                    f"`init_code` factory-address={decoded_init_code.factory_address} is not initialized"
+                )
+
+            if decoded_init_code.expected_address != safe_address:
+                raise ValidationError(
+                    f"Provided safe-address={safe_address} does not match "
+                    f"calculated-safe-address={decoded_init_code.expected_address}"
+                )
+            # Store owners used for deployment, to do checks afterward
+            self._deployment_owners = decoded_init_code.owners
+        elif not safe_is_deployed:
+            raise ValidationError(
+                "`init_code` was not provided and contract was not initialized"
+            )
+
         return init_code
 
     def validate_module_address(

safe_transaction_service/account_abstraction/tests/test_helpers.py

@@ -0,0 +1,38 @@
+from django.test import TestCase
+
+from hexbytes import HexBytes
+
+from gnosis.eth.tests.mocks.mock_bundler import user_operation_mock
+from gnosis.safe.tests.safe_test_case import SafeTestCaseMixin
+
+from ..helpers import DecodedInitCode, decode_init_code
+
+
+class TestAccountAbstractionHelpers(SafeTestCaseMixin, TestCase):
+    def test_decode_init_code(self):
+        with self.assertRaises(ValueError):
+            decode_init_code(b"", self.ethereum_client)
+
+        expected = DecodedInitCode(
+            factory_address="0x4e1DCf7AD4e460CfD30791CCC4F9c8a4f820ec67",
+            factory_data=HexBytes(
+                "0x1688f0b900000000000000000000000029fcb43b46531bca003ddc8fcb67ffe91900c7620000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001e4b63e800d000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000010000000000000000000000008ecd4ec46d4d2a6b64fe960b3d64e8b94b2234eb0000000000000000000000000000000000000000000000000000000000000140000000000000000000000000a581c4a4db7175302464ff3c06380bc3270b403700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000005ac255889882acd3da2aa939679e3f3d4cea221e00000000000000000000000000000000000000000000000000000000000000648d0dc49f00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a581c4a4db7175302464ff3c06380bc3270b40370000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
+            ),
+            initializer=b"\xb6>\x80\r\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x8e\xcdN\xc4mM*kd\xfe\x96\x0b=d\xe8\xb9K\"4\xeb\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01@\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa5\x81\xc4\xa4\xdbqu0$d\xff<\x068\x0b\xc3'\x0b@7\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00Z\xc2U\x88\x98\x82\xac\xd3\xda*\xa99g\x9e?=L\xea\"\x1e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00d\x8d\r\xc4\x9f\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa5\x81\xc4\xa4\xdbqu0$d\xff<\x068\x0b\xc3'\x0b@7\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+            singleton="0x29fcB43b46531BcA003ddC8FCB67FFE91900C762",
+            salt_nonce=0,
+            expected_address="0xB0B5c0578Aa134b0496a6C0e51A7aae47C522861",
+            owners=["0x5aC255889882aCd3da2aA939679E3f3d4cea221e"],
+            threshold=1,
+            to="0x8EcD4ec46D4D2a6B64fE960B3D64e8B94B2234eb",
+            data=b"\x8d\r\xc4\x9f\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa5\x81\xc4\xa4\xdbqu0$d\xff<\x068\x0b\xc3'\x0b@7",
+            fallback_handler="0xa581c4A4DB7175302464fF3C06380BC3270b4037",
+            payment_token="0x0000000000000000000000000000000000000000",
+            payment=0,
+            payment_receiver="0x0000000000000000000000000000000000000000",
+        )
+        result = decode_init_code(
+            HexBytes(user_operation_mock["result"]["userOperation"]["initCode"]),
+            self.ethereum_client,
+        )
+        self.assertEqual(result, expected)

safe_transaction_service/account_abstraction/tests/test_views.py

@@ -24,13 +24,16 @@
     safe_4337_user_operation_hash_mock,
     user_operation_mock,
 )
+from gnosis.eth.utils import fast_to_checksum_address
 from gnosis.safe.account_abstraction import SafeOperation as SafeOperationClass
+from gnosis.safe.proxy_factory import ProxyFactoryV141
 from gnosis.safe.safe_signature import SafeSignatureEOA
 from gnosis.safe.tests.safe_test_case import SafeTestCaseMixin
 
 from safe_transaction_service.utils.utils import datetime_to_str
 
 from .. import models
+from ..serializers import SafeOperationSerializer
 from . import factories
 
 logger = logging.getLogger(__name__)
@@ -187,8 +190,10 @@ def test_safe_operations_view(self):
             {"count": 1, "next": None, "previous": None, "results": [expected]},
         )
 
-    @mock.patch(
-        "safe_transaction_service.account_abstraction.serializers.get_safe_owners",
+    @mock.patch.object(
+        SafeOperationSerializer,
+        "_get_owners",
+        autospec=True,
     )
     @mock.patch.object(
         EthereumClient,
@@ -199,6 +204,7 @@ def test_safe_operations_view(self):
     def test_safe_operation_create_view(
         self, get_chain_id_mock: MagicMock, get_owners_mock: MagicMock
     ):
+        self.maxDiff = None
         account = Account.create()
         safe_address = safe_4337_address
         user_operation_hash = safe_4337_user_operation_hash_mock
@@ -275,7 +281,8 @@ def test_safe_operation_create_view(
                 data=data,
             )
             self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(
+            self.maxDiff = None
+            self.assertDictEqual(
                 response.data,
                 {
                     "non_field_errors": [
@@ -288,27 +295,71 @@ def test_safe_operation_create_view(
             )
 
         # Fake that Safe contract was already deployed, so `init_code` should not be provided
+        with mock.patch.object(EthereumClient, "is_contract", return_value=True):
+            response = self.client.post(
+                reverse("v1:account_abstraction:safe-operations", args=(safe_address,)),
+                format="json",
+                data=data,
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+            self.assertDictEqual(
+                response.data,
+                {
+                    "init_code": [
+                        ErrorDetail(
+                            string="`init_code` must be empty as the contract was already initialized",
+                            code="invalid",
+                        )
+                    ]
+                },
+            )
+
         with mock.patch.object(
-            EthereumClient, "is_contract", autospec=True, return_value=True
+            ProxyFactoryV141, "calculate_proxy_address", return_value=NULL_ADDRESS
         ):
             response = self.client.post(
                 reverse("v1:account_abstraction:safe-operations", args=(safe_address,)),
                 format="json",
                 data=data,
             )
             self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(
+            self.assertDictEqual(
                 response.data,
                 {
                     "init_code": [
                         ErrorDetail(
-                            string="`init_code` must be empty as the contract was already initialized",
+                            string=f"Provided safe-address={safe_address} does not match calculated-safe-address={NULL_ADDRESS}",
                             code="invalid",
                         )
                     ]
                 },
             )
 
+        # Fake that contract was not deployed and init_code was not provided
+        with mock.patch.object(
+            EthereumClient, "is_contract", return_value=False
+        ) as is_contract_mock:
+            data_without_init_code = dict(data)
+            data_without_init_code["init_code"] = None
+            response = self.client.post(
+                reverse("v1:account_abstraction:safe-operations", args=(safe_address,)),
+                format="json",
+                data=data_without_init_code,
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+            self.assertDictEqual(
+                response.data,
+                {
+                    "init_code": [
+                        ErrorDetail(
+                            string="`init_code` was not provided and contract was not initialized",
+                            code="invalid",
+                        )
+                    ]
+                },
+            )
+            is_contract_mock.assert_called_once_with(safe_address)
+
         response = self.client.post(
             reverse("v1:account_abstraction:safe-operations", args=(safe_address,)),
             format="json",
@@ -334,7 +385,7 @@ def test_safe_operation_create_view(
             data=data,
         )
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
+        self.assertDictEqual(
             response.data,
             {
                 "non_field_errors": [
@@ -357,7 +408,7 @@ def test_safe_operation_create_view(
             data=data,
         )
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
+        self.assertDictEqual(
             response.data,
             {
                 "nonce": [
@@ -369,8 +420,10 @@ def test_safe_operation_create_view(
             },
         )
 
-    @mock.patch(
-        "safe_transaction_service.account_abstraction.serializers.get_safe_owners",
+    @mock.patch.object(
+        SafeOperationSerializer,
+        "_get_owners",
+        autospec=True,
     )
     @mock.patch.object(
         EthereumClient,
@@ -465,8 +518,10 @@ def test_safe_operation_valid_until_create_view(
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-    @mock.patch(
-        "safe_transaction_service.account_abstraction.serializers.get_safe_owners",
+    @mock.patch.object(
+        SafeOperationSerializer,
+        "_get_owners",
+        autospec=True,
     )
     @mock.patch.object(
         EthereumClient,
@@ -567,12 +622,19 @@ def test_safe_operation_paymaster_and_data_create_view(
         with mock.patch.object(
             EthereumClient,
             "is_contract",
-            autospec=True,
-            side_effect=[False, True],
-        ):
+            side_effect=[False, True, True],
+        ) as is_contract_mock:
             response = self.client.post(
                 reverse("v1:account_abstraction:safe-operations", args=(safe_address,)),
                 format="json",
                 data=data,
             )
             self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+            self.assertListEqual(
+                is_contract_mock.call_args_list,
+                [
+                    mock.call(safe_address),
+                    mock.call(fast_to_checksum_address(user_operation.init_code[:20])),
+                    mock.call(paymaster_address),
+                ],
+            )