code-423n4 / 2023-01-biconomy-findings

7 stars 9 forks source link

Replay attack on different batchId #485

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L194 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L212 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L218

Vulnerability details

Description

The execTransaction function includes an input parameter called batchId that is used to determine the nonce which is included in the data signed by the owner. However, batchId is not part of the signed data. This allows any third party to replay a signed transaction using a different batchId value, as the nonce values will be the same.

Impact

Possibility of replay of any transaction for which there exists a batchId with corresponding nonce value.

Recommended Mitigation Steps

Add batchId into Transaction struct and include its value into the tx hash preimage in encodeTransactionData function.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as primary issue

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor confirmed

livingrockrises commented 1 year ago

486 is not duplicate of this primary issue.

livingrockrises commented 1 year ago

367 is not duplicate of this primary issue.

livingrockrises commented 1 year ago

233 is not related to this primary issue.

livingrockrises commented 1 year ago

175 is a different primary issue.

livingrockrises commented 1 year ago

168 is not duplicate of this primary issue.

gzeon-c4 commented 1 year ago

FYI you should look for the "MARKED DUPLICATES" section on the bottom, not sure why you quoting some issue I didn't mark as duplicate of this.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as satisfactory

c4-judge commented 1 year ago

gzeon-c4 marked issue #36 as primary and marked this issue as a duplicate of 36