safe-global / safe-transaction-service

Keeps track of transactions sent via Safe contacts and confirmed transactions. It also keeps track of Ether and ERC20 token transfers to Safe contracts.
MIT License
189 stars 256 forks source link

Service Fails to verify & store EIP1271 Signature #1726

Closed l3wi closed 8 months ago

l3wi commented 10 months ago

Describe the bug When using an EIP1271 verifier for a Safe, the transaction service wont accept the signature bytes for the contract as valid. However when the same signatures are executed onchain the transaction is successful. It seems theres a validation error.

To Reproduce Safe: here Transaction in question: here Successful tx with same signers: here

Steps to reproduce the behavior:

  1. Create & sign a new transaction on a Safe.
  2. Generate EIP1721 signature.
  3. Send to service using SafeAPIKit:
    "0x000000000000000000000000820ba510d0c2f72bbdecd7ac825a5929ea6c341a000000000000000000000000000000000000000000000000000000000000008200000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000024dde92378e6630a91d7c76b6cd4be997c1f9c2cabf684eead3a9b70245fe0ddf3e79c59ce0b45105571179e81fc5f086e60002f5b674985a0e2908042093da3d7000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f37b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2257496b7369503748573937796b624d7a786c6772436f4145665f555275336d654d526e35443176746b7177222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73652c226f746865725f6b6579735f63616e5f62655f61646465645f68657265223a22646f206e6f7420636f6d7061726520636c69656e74446174614a534f4e20616761696e737420612074656d706c6174652e205365652068747470733a2f2f676f6f2e676c2f796162506578227d0000000000000000000000001b"
  4. Service returns 400 Error:
    {
    "signature": [
        "Signature=0x000000000000000000000000820ba510d0c2f72bbdecd7ac825a5929ea6c341a000000000000000000000000000000000000000000000000000000000000008200 for owner=0x820Ba510D0C2F72BbDecd7ac825A5929EA6C341a is not valid"
      ]
    }

Expected behavior After I've generated a valid signature, I should be able to post it to the transaction service. The service should correctly validate it & store the signature for execution by another wallet.

Environment:

l3wi commented 10 months ago

Happy to provide further examples or information if required!

mmv08 commented 9 months ago

So, I did some debugging, and to me, it doesn't seem like your signature is valid, let's go through this step by step:

  1. Signature verifier, first 32 bytes, we get
    000000000000000000000000820ba510d0c2f72bbdecd7ac825a5929ea6c341a
  2. Signature start position, second 32 bytes, we get
    0000000000000000000000000000000000000000000000000000000000000082
    0x82 = 130 in decimal
  3. Signature type, last 1 byte
    00
  4. Signature length, 32 bytes from the signature start position
    0000000000000000000000000000000000000000000000000000000000000220
    0x220 = 544
  5. Signature, obtained from signature, starting from 130 + 32 (for signature size) till 130 + 32 + 544
    00000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000024dde92378e6630a91d7c76b6cd4be997c1f9c2cabf684eead3a9b70245fe0ddf3e79c59ce0b45105571179e81fc5f086e60002f5b674985a0e2908042093da3d7000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f37b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2257496b7369503748573937796b624d7a786c6772436f4145665f555275336d654d526e35443176746b7177222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73652c226f746865725f6b6579735f63616e5f62655f61646465645f68657265223a22646f206e6f7420636f6d7061726520636c69656e74446174614a534f4e20616761696e737420612074656d706c6174652e205365652068747470733a2f2f676f6f2e676c2f796162506578227d0000000000000000000000001b

I get these signature bytes and validate them with your signer contract:

Screenshot 2023-11-18 at 22 32 11

So far, the backend response seems correct on my end. Please let me know if I missed anything

mmv08 commented 9 months ago

Also, in the Safe you referenced, I can see a contract signature successfully registered in the service:

https://safe-transaction-optimism.safe.global/api/v1/multisig-transactions/0x787727e479b264ae8e54defa2f86c2a32e098bd923b3f6964b7d4b4ed309588e/

mmv08 commented 9 months ago

Signature start position, second 32 bytes, we get 0000000000000000000000000000000000000000000000000000000000000082 0x82 = 130 in decimal

Actually, shouldn't this be 65? Seems like it's accounted for hex encoding 🤔

l3wi commented 9 months ago

Hey @mmv08 thanks for getting back to me.

I've been able to get you a better example to look at:

Valid Signature:

Here is a valid signature that the Safe transaction service rejects:

0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd00000000000000000000000000000000000000000000000000000000000000820000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000002470c46173d5e799fecce5ad97ac98228fe509b2f3b104c52b009e79fc9788aa9c319e9f9136831f3fc5d895e494eb15655d66102e5f2a4bc421865c0499e483ea000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000867b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2265324744736552664858317764796e333867415073544d657a645f33717153726c453367417835357a4c6f222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73657d000000000000000000000000000000000000000000000000001b

Here is a Tenderly simulation confirming that signature executes successfully

Safe TX Service

Request URL:

https://safe-transaction-arbitrum.safe.global//api/v1/multisig-transactions/0x7b6183b1e45f1d7d707729f7f2000fb1331ecddff7aaa4ab944de0031e79ccba/confirmations/

Payload:

{
    "signature": "0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd00000000000000000000000000000000000000000000000000000000000000820000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000002470c46173d5e799fecce5ad97ac98228fe509b2f3b104c52b009e79fc9788aa9c319e9f9136831f3fc5d895e494eb15655d66102e5f2a4bc421865c0499e483ea000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000867b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2265324744736552664858317764796e333867415073544d657a645f33717153726c453367417835357a4c6f222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73657d000000000000000000000000000000000000000000000000001b"
}

Response:

{
    "signature": [
        "Signature=0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd000000000000000000000000000000000000000000000000000000000000008200 for owner=0x1DeA27210BeA53cF42c34705DdD63B37e4FBd2BD is not valid"
    ]
}

What I think is happening:

If you check the payload of the STATICCALL in Tenderly from Gnosis to the verifier contract, you'll see that its calling the verify function thats using (bytes, bytes) instead of (bytes32, bytes). This is why your attempt to call the contract with the bytes32 hash failed as it doesn't mimic the way the Safe works. I imagine the Safe TX service does the same thing when validating the tx.

Side note:

To your point about the signature offset, the signature fails the simulation if I don't use do 65 * 2, aka offset * signature number (2). I'm not sure why that is 🤔 .

mmv08 commented 9 months ago

To your point about the signature offset, the signature fails the simulation if I don't use do 65 2, aka offset signature number (2). I'm not sure why that is 🤔 .

Hmm, I thought that in the original issue the amount of signatures was 1.

l3wi commented 9 months ago

There were two: an EOA & a Contract Signature.

I linked to the pending tx in the first message: here

mmv08 commented 9 months ago

Hey @mmv08 thanks for getting back to me.

I've been able to get you a better example to look at:

Valid Signature:

Here is a valid signature that the Safe transaction service rejects:

0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd00000000000000000000000000000000000000000000000000000000000000820000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000002470c46173d5e799fecce5ad97ac98228fe509b2f3b104c52b009e79fc9788aa9c319e9f9136831f3fc5d895e494eb15655d66102e5f2a4bc421865c0499e483ea000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000867b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2265324744736552664858317764796e333867415073544d657a645f33717153726c453367417835357a4c6f222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73657d000000000000000000000000000000000000000000000000001b

Here is a Tenderly simulation confirming that signature executes successfully

Safe TX Service

Request URL:

https://safe-transaction-arbitrum.safe.global//api/v1/multisig-transactions/0x7b6183b1e45f1d7d707729f7f2000fb1331ecddff7aaa4ab944de0031e79ccba/confirmations/

Payload:

{
    "signature": "0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd00000000000000000000000000000000000000000000000000000000000000820000000000000000000000000000000000000000000000000000000000000001c000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000002470c46173d5e799fecce5ad97ac98228fe509b2f3b104c52b009e79fc9788aa9c319e9f9136831f3fc5d895e494eb15655d66102e5f2a4bc421865c0499e483ea000000000000000000000000000000000000000000000000000000000000002549960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d9763050000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000867b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a2265324744736552664858317764796e333867415073544d657a645f33717153726c453367417835357a4c6f222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a33303030222c2263726f73734f726967696e223a66616c73657d000000000000000000000000000000000000000000000000001b"
}

Response:

{
    "signature": [
        "Signature=0x0000000000000000000000001dea27210bea53cf42c34705ddd63b37e4fbd2bd000000000000000000000000000000000000000000000000000000000000008200 for owner=0x1DeA27210BeA53cF42c34705DdD63B37e4FBd2BD is not valid"
    ]
}

What I think is happening:

If you check the payload of the STATICCALL in Tenderly from Gnosis to the verifier contract, you'll see that its calling the verify function thats using (bytes, bytes) instead of (bytes32, bytes). This is why your attempt to call the contract with the bytes32 hash failed as it doesn't mimic the way the Safe works. I imagine the Safe TX service does the same thing when validating the tx.

Side note:

To your point about the signature offset, the signature fails the simulation if I don't use do 65 * 2, aka offset * signature number (2). I'm not sure why that is 🤔 .

I think you're right! The service verifies the signature using the safe transaction hash, but in the safe contract it uses the pre-image transaction data because of the legacy EIP-1271 method. I'll see what we can do.

l3wi commented 9 months ago

Awesome! Thanks for looking into this.

l3wi commented 9 months ago

Hey @mmv08 any update here?

mmv08 commented 9 months ago

Hey @mmv08 any update here?

Hey, apologies, I was on vacation. I will check if someone from the backend team can look into the issue this week. Otherwise, I will have time to look into it myself on Friday.

mmv08 commented 8 months ago

@Uxio0 should this ticket be reopened as discussed in slack?

Uxio0 commented 8 months ago

@mmv08 we can reopen as the fix is still not in production

l3wi commented 8 months ago

Great to see progress here. Thanks so much for getting the fix done.

Looking forward to seeing it go live

Uxio0 commented 8 months ago

This is now fixed on production