[bug] Signed payload could be submitted for unintended extrinsic

[JoeCap08055]

Description

There are a number of extrinsics requiring signed payloads that share the same payload type. It is possible that a user could sign a payload intended for Extrinsic A, but the provider could instead (whether maliciously or not) execute Extrinsic B. In some cases this can have consequences that the user signing the payload did not intend.

Currently, there seem to be 2 pairs of extrinsics that share payload types:

• handles::claim_handle and handles::change_handle both accept a signed ClaimHandlePayload payload
• msa::create_sponsored_account_with_delegation and msa::grant_delegation both accept a signed AddProvider payload

In the case of the msa pallet, there do not seem to be any unintended consequences that can result from executing the incorrect/unintended extrinsic, as the two calls are mutually exclusive (one can only succeed if there is no MSA associated with the delegator key; the other can only succeed if there is an MSA associated with the delegator key).

However, in the case of the handles pallet, unintended consequences may result. A user that is expecting a claim_handle extrinsic to be executed does not expect an existing handle (if it exists) to be retired; rather, they would expect the extrinsic to fail. But if change_handle is executed instead, with the same payload, the existing handle would be retired. Likewise, if the user expected change_handle to be called, they would not expect a handle to be applied if there was not already an existing handle; but if claim_handle was executed instead, the handle would be claimed on an account that did not previously have one.

Granted, these are not critical use cases, and it is unlikely that a user would sign a payload with the expectation that the extrinsic would fail; however, there may be other extrinsics in the future that share a signed payload structure, and we should identify a solution for dealing with them.

Discussion

~Rejected Solution~

I thought about simply having the user providing an encoded extrinsic call for the provider to execute, instead of just a signed payload & having the provider create the extrinsic call. But that is not secure; a malicious provider could simply decode the extrinsic, extract the payload and signature, and submit it in an unintended extrinsic.

Proposed Solution # 1

One possible solution would be to have all signed payloads include the pallet and extrinsic index. This could be checked near the top of the extrinsic (or perhaps in a signed extension?) and an error thrown if the pallet and extrinsic signed in the payload do not match the pallet and extrinsic being executed.

• The encoding of the pallet and extrinsic indices would only add a few bytes to the payload, and would not incur any additional storage reads or writes to check. The check could even be done before validating the signature itself.
• If we could put this logic in a signed extension, it could return an RpcError and would not incur a cost
• Whether the logic is in a signed extension or not, as long as this check occurs before the payload signature is registered in the signature registry, the provider could re-submit using the correct intended extrinsic (as long as it hasn't expired)

Other possible solutions... ?

[JoeCap08055]

Thanks to @shannonwells for helping me realize that this is indeed an issue worth raising.

[wilwade]

2024-11-21 Community Call Notes

• How would the user verify and know?
  • Chain explorers?
  • Wallet integrations?
  • EIP-712
• MSA create/grant is intended to be the same to support the race condition
  • Should those extrinsics be merged?
  • (For these) no impact to the user, but the provider could lose some capacity

Potential Action Items

1. We should be careful reusing signature structures
2. We should consider merging extrinsics that are idempotent from the user's perspective
   • Consider using a state based hash (ala stateful storage patterns) to ensure the user's desired result
3. We should consider including some data point to indicate the user's desired outcome over just the pure data structure

Should we consider changing how a user's actions are processed

• Now: Different ways optimized for each setup
• Option: Proxy pattern?
  • One entry point that does the validation of authority, then it dispatches to the correct pallet
  • State based validation?