The withdraw function in FungibleAssetVaultForDAO.sol is using payable.transfer() for transferring native ETH.
if (collateralAsset == ETH) payable(msg.sender).transfer(amount);
This is unsafe as transfer has hard coded gas budget and can fail when the user/caller is a smart contract.
There is a high probability that the members of the WHITELISTED_ROLE is another smart contract.
Whenever the calling smart contract either fails to implement the payable fallback function or cumulative gas cost of the function sequence invoked on the native token transfer exceeds 2300 gas consumption limit, the native ETH tokens sent end up undelivered and the withdraw functionality will fail each time.
Impact
Under the above mentioned circumstances, the smart contract cannot withdraw the native collateral amount, unless the smartcontract is deployed as upgradable, and corresponding changes are effected, hence marking as high.
Recommended Mitigation Steps
In the withdraw function, the transfer() can be replaced using low-level call.value(amount) with the corresponding result check.
Lines of code
https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L201
Vulnerability details
The withdraw function in FungibleAssetVaultForDAO.sol is using payable.transfer() for transferring native ETH.
This is unsafe as transfer has hard coded gas budget and can fail when the user/caller is a smart contract. There is a high probability that the members of the WHITELISTED_ROLE is another smart contract.
Whenever the calling smart contract either fails to implement the payable fallback function or cumulative gas cost of the function sequence invoked on the native token transfer exceeds 2300 gas consumption limit, the native ETH tokens sent end up undelivered and the withdraw functionality will fail each time.
Impact
Under the above mentioned circumstances, the smart contract cannot withdraw the native collateral amount, unless the smartcontract is deployed as upgradable, and corresponding changes are effected, hence marking as high.
Recommended Mitigation Steps
In the withdraw function, the transfer() can be replaced using low-level call.value(amount) with the corresponding result check.