VIS-2 / taobank-04-24

0 stars 0 forks source link

User can lose money paying someone else's debt #3

Open 0xMilenov opened 5 months ago

0xMilenov commented 5 months ago

Impact

Severity: High Likelihood: High

Context

VaultFactory::repay()

Description

The repay function lacks checks to ensure that the caller is the vault owner, allowing any user to repay any vault's debt. This can result in users paying off debts for others without receiving any collateral in return.

function repay(address _vault, uint256 _amount) external {
    require(containsVault(_vault), 'vault-not-found');
    totalDebt -= _amount;
    Vault(_vault).repay(_amount);

    IMintableToken(stable).safeTransferFrom(
        _msgSender(),
        address(this),
        _amount
    );
    IMintableToken(stable).burn(_amount);
}

Recommendation

Add an onlyVaultOwnerOrOperator(_vault) modifier to restrict repayments to the owner or authorized users only.

- function repay(address _vault, uint256 _amount) external {
+ function repay(address _vault, uint256 _amount) external onlyVaultOwnerOrOperator(_vault) {
    require(containsVault(_vault), 'vault-not-found');
    totalDebt -= _amount;
    Vault(_vault).repay(_amount);

    IMintableToken(stable).safeTransferFrom(
        _msgSender(),
        address(this),
        _amount
    );
    IMintableToken(stable).burn(_amount);
}