search

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

13 stars 10 forks source link

```execute()``` and its related functions revert for ```msg.value```> 0 #468

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/base/ModuleManager.sol#L61-L73 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L460-L463 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L449-L453 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L489-L492

Vulnerability details

Impact

execTransactionFromModule() will revert for msg.value > 0 as it lacks the payable modifier

Proof of Concept

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/base/ModuleManager.sol#L61-L73 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L460-L463 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L449-L453 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L489-L492

Tools Used

Manual Review

Recommended Mitigation Steps

Add payable modifier

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor disputed

livingrockrises commented 1 year ago

funds are supposed to flow from the wallet and not the caller

livingrockrises commented 1 year ago

this is not duplicate of #460