sherlock-audit / 2022-09-knox-judging

0 stars 0 forks source link

berndartmueller - Vault does not fully conform to `EIP4626` #134

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

berndartmueller

medium

Vault does not fully conform to EIP4626

Summary

The VaultBase contract does not fully conform to the EIP4626 standard.

Vulnerability Detail

The VaultBase contract implements the EIP4626 standard (EIP-4626: Tokenized Vault Standard).

However, according to EIP4626, the below-mentioned functions do not fully adhere to the specs.

ERC4626Base.maxWithdraw (Found in @solidstate/contracts/token/ERC4626/base/ERC4626Base.sol)

function maxWithdraw(address owner)
    external
    view
    returns (uint256 maxAssets)
{
    maxAssets = _maxWithdraw(owner); // @audit-info `_maxWithdraw` is the default implementation in @solidstate's ERC4626BaseInternal.sol and not overwritten by Knox
}
  1. MUST factor in global and user-specific limits, if deposits are entirely disabled (even temporarily) it MUST return 0. The vault has a withdrawal lock which is active after the auction has started and deactivated when the auction is processed. Therefore, this withdrawal lock state should be reflected in the maxWithdraw function to be compliant.

ERC4626Base.maxRedeem (Found in @solidstate/contracts/token/ERC4626/base/ERC4626Base.sol)

function maxRedeem(address owner)
    external
    view
    returns (uint256 maxShares)
{
    maxShares = _maxRedeem(owner); // @audit-info `_maxRedeem` is the default implementation in @solidstate's ERC4626BaseInternal.sol and not overwritten by Knox
}
  1. MUST factor in global and user-specific limits, if deposits are entirely disabled (even temporarily) it MUST return 0. The vault has a withdrawal lock which is active after the auction has started and deactivated when the auction is processed. Therefore, to be compliant, this withdrawal lock state should be reflected in the maxRedeem function.

Impact

Not adhering to the EIP4626 standard could lead to integration issues with other protocols.

Code Snippet

See above.

Tool Used

Manual review

Recommendation

Consider implementing the EIP4626 standard as close to the specs as possible.

Duplicate of #57