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

6 stars 8 forks source link

Unpreparedness for upgrades contracts that `SmartAccount` inherits #501

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/SmartAccount.sol#L18

Vulnerability details

Description

The SmartAccount contract inherits many contracts, some of which have their own storage management logic. In case of an upgrade, adding new storage variables to the inherited contracts will colapse the storage layout. This will create an inconvenience for updates with possible vulnerabilities on storage overlaps.

Impact

Deterioration of code architecture in case of an upgrade that creates storage overlaps.

Recommended Mitigation Steps

Use a base contract for all storage management logic related to the SmartAccount. Another way to solve this is to add a storage gap to all contracts that SmartAccount inherits.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #352

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor confirmed

c4-judge commented 1 year ago

gzeon-c4 marked the issue as satisfactory