Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

duc - buyCollateral function pass a false value of skim param for adding collateral #57

Closed
sherlock-admin3 opened this issue Mar 15, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Mar 15, 2024

duc

high

buyCollateral function pass a false value of skim param for adding collateral

Summary

In the buyCollateral function of BBLeverage or SGLLeverage, collateral tokens are deposited to YieldBox for the contract. However, this function calls _addCollateral with skim set to false, which will pull collateral from the user's YieldBox position.

Vulnerability Detail

buyCollateral function is used to borrow assets and swap to add collateral into the market. Although it already receives collateral shares of YieldBox after depositing, it calls _addCollateral with the skim parameter set to false.

uint256 collateralShare = yieldBox.toShare(collateralId, amountOut, false);
address(asset).safeApprove(address(yieldBox), type(uint256).max);
yieldBox.depositAsset(collateralId, address(this), address(this), 0, collateralShare); // TODO Check for rounding attack?
...
_addCollateral(calldata_.from, calldata_.from, false, 0, collateralShare);

This results in collateral being incorrectly pulled from the user, even though the necessary tokens are already present in the contract.

function _addTokens(address from, uint256 _tokenId, uint256 share, uint256 total, bool skim) internal {
    if (skim) {
        require(share <= yieldBox.balanceOf(address(this), _tokenId) - total, "BB: too much");
    } else {
        // yieldBox.transfer(from, address(this), _tokenId, share);
        bool isErr = pearlmit.transferFromERC1155(from, address(this), address(yieldBox), _tokenId, share);
        if (isErr) {
            revert TransferFailed();
        }
    }
}

Impact

Users will get losses of funds when using the buyCollateral function if they have enough collateral shares and allowance.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L109
https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/singularity/SGLLeverage.sol#L92

Tool used

Manual Review

Recommendation

buyCollateral function should call _addCollateral with the skim parameter set to true.

Duplicate of #139

@sherlock-admin2 sherlock-admin2 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Mar 15, 2024
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Mar 20, 2024
@nevillehuang nevillehuang added Medium A valid Medium severity issue and removed High A valid High severity issue labels Mar 28, 2024
@sherlock-admin3 sherlock-admin3 changed the title Winning Cobalt Barracuda - buyCollateral function pass a false value of skim param for adding collateral duc - buyCollateral function pass a false value of skim param for adding collateral Mar 31, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants