search

code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

`Vault.balance()` mixes normalized and standard amounts #132

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The Vault.balance function uses the balanceOfThis function which scales ("normalizes") all balances to 18 decimals.

for (uint8 i; i < _tokens.length; i++) {
    address _token = _tokens[i];
    // everything is padded to 18 decimals
    _balance = _balance.add(_normalizeDecimals(_token, IERC20(_token).balanceOf(address(this))));
}

Note that balance()'s second term IController(manager.controllers(address(this))).balanceOf() is not normalized. The code is adding a non-normalized amount (for example 6 decimals only for USDC) to a normalized (18 decimals).

Impact

The result is that the balance() will be under-reported. This leads to receiving wrong shares when depositing tokens, and a wrong amount when redeeming tokens.

Recommended Mitigation Steps

The second term IController(manager.controllers(address(this))).balanceOf() must also be normalized before adding it. IController(manager.controllers(address(this))).balanceOf() uses _vaultDetails[msg.sender].balance which directly uses the raw token amounts which are not normalized.

GalloDaSballo commented 2 years ago

balance and balanceOfThis mixes the usage of decimals by alternatingly using _normalizeDecimals This can break accounting as well as create opportunities for abuse A consistent usage of _normalizeDecimals would mitigate