Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Signed payload could be submitted for unintended extrinsic #1892

Open
JoeCap08055 opened this issue Feb 26, 2024 · 1 comment
Open

Comments

@JoeCap08055
Copy link
Collaborator

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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant