Skip to content

Latest commit

 

History

History
60 lines (47 loc) · 3.08 KB

016.md

File metadata and controls

60 lines (47 loc) · 3.08 KB
Raw

Bauer

high

A bad actor can steal funds from users

Summary

The updateCommitment() function does not require that the ledner not be updated, so a bad lender can calls this function to update the lender to user who approved the CollateralManager contract to spend his funds. By doing so ,a bad actor can steal funds from users

Vulnerability Detail

The LenderCommitmentForwarder protocol allows user to creates a loan commitment from a lender for a market. Inside the createCommitment() function ,the protocol will store lender information into the commitments[commitmentId_] mapping. The function updateCommitment() updates a commitment with a given ID and new commitment data. It can only be called by the lender of the commitment. The function first checks if the new principal token address and market ID match the existing commitment's data, as they cannot be updated. If the check passes, the commitment is updated with the new data, and the function then calls another function to validate the commitment's new values. Based on these check conditions, we found that we can also update the ledner.Here is the problem. Let me show how a bad actor can steal funds from users. 1.Bob creates a fake token as collateral. 2.Next he finds user Alice who approved the CollateralManager contract to spend his funds and still has funds in the account (e.g. WETH) 3.Then, he calls the createCommitment() function with a fake token as collateral and WETH as the principal token. 4.Since the updateCommitment() function does not require that the ledner not be updated, so Bob calls this function to update the lender to Alice. 5.By doing so, Bob calls the acceptCommitment() to accept the commitment to submitBid and acceptBid. The protocol will transfer fake tokens to CollateralEscrow contract and transfer WETH from Alice to Bob.And Alice was completely unaware that she had provided the lending. Bob successfully stole funds from users.

function updateCommitment(
        uint256 _commitmentId,
        Commitment calldata _commitment
    ) public commitmentLender(_commitmentId) {
        require(
            _commitment.principalTokenAddress ==
                commitments[_commitmentId].principalTokenAddress,
            "Principal token address cannot be updated."
        );
        require(
            _commitment.marketId == commitments[_commitmentId].marketId,
            "Market Id cannot be updated."
        );

        commitments[_commitmentId] = _commitment;

        validateCommitment(commitments[_commitmentId]);

        emit UpdatedCommitment(
            _commitmentId,
            _commitment.lender,
            _commitment.marketId,
            _commitment.principalTokenAddress,
            _commitment.maxPrincipal
        );
    }

Impact

A bad actor can steal funds from users.

Code Snippet

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/LenderCommitmentForwarder.sol#L208-L233

Tool used

Manual Review

Recommendation

Inside the updateCommitment() function, make sure that lender cannot be updated