withdraw() uses payable(msg.sender).transfer() which should be avoided
In FungibleAssetVaultForDAO.sol, withdraw () uses the transfer function to send funds when the collateralAsset is Eth. But, this is not recommended especially if the call is made by a smart contract as its fallback will consume more than the 2300 gas stipend:
1.Incorrect spelling
https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L67
https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L888
https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L928
In FungibleAssetVaultForDAO.sol, withdraw () uses the transfer function to send funds when the collateralAsset is Eth. But, this is not recommended especially if the call is made by a smart contract as its fallback will consume more than the 2300 gas stipend:
https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L201
A similar finding was made for Open Leverage in this report :
https://github.com/code-423n4/2022-01-openleverage-findings/issues/75
When mentioned, #sphagetti dev stated :
‘in our case it still works since gnosis safes consume less than 2300 gas when receiving eth since the receive function is empty’.