code-423n4 / 2023-09-asymmetry-findings

2 stars 1 forks source link

Missing `__ERC20_init()` call in `AfEth`'s `initialize()` function #27

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L72-L75

Vulnerability details

Bug Description

The AfEth contract inherits Openzeppelin's ERC20Upgradeable contract:

AfEth.sol#L10

contract AfEth is Initializable, OwnableUpgradeable, ERC20Upgradeable {

However, its initialize() function does not call __ERC20_init(), which is used to initialize the name and symbol of the contract:

ERC20Upgradeable.sol#L55-L62

    function __ERC20_init(string memory name_, string memory symbol_) internal onlyInitializing {
        __ERC20_init_unchained(name_, symbol_);
    }

    function __ERC20_init_unchained(string memory name_, string memory symbol_) internal onlyInitializing {
        _name = name_;
        _symbol = symbol_;
    }

Therefore, even after AfEth is deployed and initialized, its name() and symbol() functions will still return empty strings.

Impact

Any contract that calls name() or symbol() for the AfEth contract will get an incorrect empty string in return, as they were never initialized.

This could break composability with other contracts, dapps or front-ends, such as Uniswap, which might expect the contract's name or symbol to be a valid string.

Recommended Mitigation

Consider adding a __ERC20_init() call in the contract's initialize() function:

AfEth.sol#L72-L75

    function initialize() external initializer {
+       __ERC20_init("Asymmetry Finance AfEth", "AfEth");        
        _transferOwnership(msg.sender);
        ratio = 5e17;
    }

Assessed type

Error

elmutt commented 1 year ago

https://github.com/asymmetryfinance/afeth/pull/158/files

c4-judge commented 1 year ago

0xleastwood marked the issue as primary issue

0xleastwood commented 1 year ago

I would argue this is QA because assets/liveness is not impacted in any way but off-chain integration is.

c4-judge commented 1 year ago

0xleastwood changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xleastwood marked the issue as grade-a

c4-sponsor commented 1 year ago

elmutt (sponsor) confirmed

c4-judge commented 1 year ago

0xleastwood removed the grade