Skip to content

Latest commit

 

History

History
91 lines (63 loc) · 5.21 KB

071.md

File metadata and controls

91 lines (63 loc) · 5.21 KB

chaduke

medium

A malicous user can manipulate the amount of collateral for a borrower.

Summary

A malicious user can manipulate the amount of a collateral for a borrower. For the case for ERC721 as collaterals, the malicious user can front-run the deployAndDeposit() with commitCollateral() and modifies the collateral amount to 1 and causes a DOS attack to deployAndDeposit().

A malicious user can also minipulate the amount of a collateral so that CollateralEscrow#withdraw() will always revert so that nobody (the borrower, the lender, the liquidator) can claim the collaterals and the collaterals will be stuck in the contract.

Vulnerability Detail

A user can commit a list of collaterals for a borrower by calling the commitCollateral() function.

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/CollateralManager.sol#L117-L130

The only requirement is that the check of checkBalances() should succeed.

Meanwhile, for each collateral, commitCollateral() calls the _commitCollateral(_bidId, info) to accomplish such commitment.

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/CollateralManager.sol#L426-L442

The problem with the _commitCollateral(_bidId, info) is that it fails to detect the existence of an existing collateral and will replace an old collateral information with a new one. In particular, at L431, the function does not check the return value of add(). As a result, the amount of an existing collateral with be replaced by a new amount.

So any malicious user can call commitCollateral() and replaces the amount of an existing collateral with a new amount, including resetting the new amount to zero as along as the amount of collateral is no more than the balance of the collateral tokens of the borrower (see the implementation of _checkBalance() (https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/CollateralManager.sol#L478-L504.

For the case for ERC721 as collaterals, the malicious user can front-run the deployAndDeposit() with commitCollateral() and modifies the collateral amount to 1 and causes a DOS attack to deployAndDeposit(). This is because for ERC721, amount must be equal to 1 to deposit successfully:

127 else if (_collateralType == CollateralType.ERC721) {
128 require(_amount == 1, "Incorrect deposit amount");
129 IERC721Upgradeable(_collateralAddress).transferFrom(
130 _msgSender(),
131 address(this),
132 _tokenId
133 );
134 }

A malicious user can also minipulate the amount of a collateral so that CollateralEscrow#withdraw() will always revert so that nobody (the borrower, the lender, the liquidator) can claim the collaterals and the collaterals will be stuck in the contract.

For example, after a bid is accepted, a malicious user can call commitCollateral() to increase the amount of collateral. As a result, the following line will fail in CollateralEscrowV1#withdraw():

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/escrow/CollateralEscrowV1.sol#L101

 collateral._amount -= _amount;

In this way, nobody (the borrower, the lender, the liquidator) can claim the collaterals and the collaterals will be stuck in the contract.

Impact

A malicious user can manipulate the amount of a collateral for a borrower, including resetting it to zero; as a result the malicious user can launch an attack for the case of ERC721 tokens as collaterals.

Code Snippet

See above

Tool used

VSCode

Manual Review

Recommendation

Check if a collateral already exists in _commitCollateral(_bidId, info), and only commits a new collateral when it does not exist yet.

or add a modifier so that commitColalteral() can only be called by TellerV2.

function _commitCollateral(
        uint256 _bidId,
        Collateral memory _collateralInfo
    ) internal virtual {
        CollateralInfo storage collateral = _bidCollaterals[_bidId];
-        collateral.collateralAddresses.add(_collateralInfo._collateralAddress);
+       bool result = collateral.collateralAddresses.add(_collateralInfo._collateralAddress);
+       if(!bool) revert cannotCommitAnExistingCollateral();
        collateral.collateralInfo[
            _collateralInfo._collateralAddress
        ] = _collateralInfo;
        emit CollateralCommitted(
            _bidId,
            _collateralInfo._collateralType,
            _collateralInfo._collateralAddress,
            _collateralInfo._amount,
            _collateralInfo._tokenId
        );
    }