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

10 stars 11 forks source link

Non upgradeable version being used #540

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/721e2afb493d8bc0bc9488b84eaf2deb14c8b43f/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L27 https://github.com/code-423n4/2023-01-biconomy/blob/7b02ebfcebbf79e6df65ee30efa347cffd28ebcd/scw-contracts/contracts/smart-contract-wallet/aa-4337/samples/SimpleAccount.sol#L9

Vulnerability details

Impact

Based on the context and comments in the code, the SimpleAccount.sol and SmartAccount.sol contract is designed to be deployed as an upgradeable proxy contract.

However, the current implementation is using an non-upgradeable version of the Initializable library: @openzeppelin/contracts/proxy/utils/Initializable.sol instead of the upgradeable version: @openzeppelin/contracts-upgradeable/contracts/proxy/utils/Initializable.sol

Code

https://github.com/code-423n4/2023-01-biconomy/blob/721e2afb493d8bc0bc9488b84eaf2deb14c8b43f/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L27 https://github.com/code-423n4/2023-01-biconomy/blob/7b02ebfcebbf79e6df65ee30efa347cffd28ebcd/scw-contracts/contracts/smart-contract-wallet/aa-4337/samples/SimpleAccount.sol#L9

Mitigation

Use the upgradeable version

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #352

livingrockrises commented 1 year ago

there isn't difference in non-upgradeable and upgradeable version mentioned by the warden

https://www.diffchecker.com/WKtI1akM/

appreciate a discussion

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

gzeon-c4 marked the issue as satisfactory