Tapioca DAO - TiesStevelink's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLCollateral.sol#L27 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLendingCommon.sol#L23-L25 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L84

Vulnerability details

Impact

Users can add any amounts of collateral without the allowing borrow permission by calling addCollateral() function with share of 0.

Proof of Concept

Let's call addCollateral() function with share of 0 in SGLCollateral.sol.

uint256 amount = 1e18 * 10000000000 // as much as possible
sglcollateral.addCollateral(from, to, false, amount, 0);

And then it will be passed allowedBorrow(from, 0) modifier, because share is 0 in addCollateral() function.

    function addCollateral(
        address from,
        address to,
        bool skim,
        uint256 amount,
        uint256 share
    ) public notPaused allowedBorrow(from, share) {
        _addCollateral(from, to, skim, amount, share);
    }

    function _addCollateral(
        address from,
        address to,
        bool skim,
        uint256 amount,
        uint256 share
    ) internal {
        if (share == 0) {
            share = yieldBox.toShare(collateralId, amount, false);
        }
        userCollateralShare[to] += share;
        ...
    }

As we can see _addCollateral() function, if share is 0, share will be calculated based on amount. And then recalculated share will be added to user's collateral.

Tools Used

Manual

Recommended Mitigation Steps

Need to add _allowedBorrow(from, share) in _addCollateral() function like the following code.

    function _addCollateral(
        address from,
        address to,
        bool skim,
        uint256 amount,
        uint256 share
    ) internal {
        if (share == 0) {
            share = yieldBox.toShare(collateralId, amount, false);
            _allowedBorrow(from, share);
        }
        userCollateralShare[to] += share;
        ...
    }

Assessed type

Context