You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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... ?
The text was updated successfully, but these errors were encountered:
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
andhandles::change_handle
both accept a signedClaimHandlePayload
payloadmsa::create_sponsored_account_with_delegation
andmsa::grant_delegation
both accept a signedAddProvider
payloadIn 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 aclaim_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 ifchange_handle
is executed instead, with the same payload, the existing handle would be retired. Likewise, if the user expectedchange_handle
to be called, they would not expect a handle to be applied if there was not already an existing handle; but ifclaim_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 SolutionI 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.
Other possible solutions... ?
The text was updated successfully, but these errors were encountered: