yieldprotocol / variable-rate-audit-obheda12

0 stars 0 forks source link

[M-07] Unintended Vault Transfer to Zero Address #8

Open obheda12 opened 1 year ago

obheda12 commented 1 year ago

Summary

The VRLadle contract contains a give() function that allows a vault owner to transfer their vault to another address:

function give(
    bytes12 vaultId_,
    address receiver
) external payable returns (VRDataTypes.Vault memory vault) {
    (bytes12 vaultId, ) = getVault(vaultId_);
    vault = cauldron.give(vaultId, receiver);
}

However, this function does not check whether the receiver is a zero address. If a vault owner accidentally transfers the vault to the zero address, they will lose the vault and its collateral forever. Furthermore, several functions will break as they call _getVault(), which requires msg.sender to be the vault owner. These functions include:

Additionally, the VRCauldron::_give() function also lacks validation for the receiver address as shown below:

function _give(
    bytes12 vaultId,
    address receiver
) internal returns (VRDataTypes.Vault memory vault) {
    require(vaultId != bytes12(0), "Vault id is zero");
    require(vaults[vaultId].baseId != bytes6(0), "Vault doesn't exist"); // Base can't take bytes6(0) as their id
    vault = vaults[vaultId];
    vault.owner = receiver;
    vaults[vaultId] = vault;
    emit VaultGiven(vaultId, receiver);
}

PoC

VRCauldron::_give() function also doesn't have checks for receiver address.

function _give(
    bytes12 vaultId,
    address receiver
) internal returns (VRDataTypes.Vault memory vault) {
    require(vaultId != bytes12(0), "Vault id is zero");
    require(vaults[vaultId].baseId != bytes6(0), "Vault doesn't exist"); // Base can't take bytes6(0) as their id
    vault = vaults[vaultId];
    vault.owner = receiver;
    vaults[vaultId] = vault;
    emit VaultGiven(vaultId, receiver);
}

Recommendations

In order to prevent a user from unintentionally transferring their vault to the zero address and causing the loss of their collateral or breaking the functionality of several functions, implement proper validation in the give function.

function give(
    bytes12 vaultId_,
    address receiver
) external payable returns (VRDataTypes.Vault memory vault) {
+   require(receiver != address(0), "...");
    (bytes12 vaultId, ) = getVault(vaultId_);
    vault = cauldron.give(vaultId, receiver);
}
iamsahu commented 1 year ago

Same as #2

obheda12 commented 1 year ago

Acknowledged - Risk is assumed, severity decrease to low