yongkiws - function RageDnDepository:deposit, redeem are not so safe without controller approval

[github-actions[bot]]

yongkiws

high

function RageDnDepository:deposit, redeem are not so safe without controller approval

Summary

function RageDnDepository::deposit, redeem mechanism to control the access and can be abused by attackers

Vulnerability Detail

because there is no mechanism to control the access rights of the user different from the controller, the attacker can deposit, redeem without the consent of the controller and the amount of assets allowed to withdraw, the amount of assets that can be done by the user, the attacker can get the amount of assets that he should not be able to. the attacker can generate a non-withdrawable amount. the attacker can redeem the amount that was not supposed to be withdrawn and cause unwanted things to happen

Impact

RageDnDepository.sol#L99 Bob and Alice can deposit assets that they shouldn't be able to, such as depositing assets in amounts that shouldn't be possible such as price manipulation As a result, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token. getDepositoryAssets , getDepositoryShares() and assetsDeposited()

    function deposit(address asset, uint256 assetAmount)
        external
        onlyController
        returns (uint256)
    {
        if (asset != assetToken) {
            revert UnsupportedAsset(asset);
        }
        netAssetDeposits += assetAmount;
        IERC20(assetToken).approve(address(vault), assetAmount);
        uint256 shares = vault.deposit(assetAmount, address(this));
        uint256 redeemableAmount = _assetsToRedeemable(assetAmount);
        redeemableUnderManagement += redeemableAmount;
        _checkSoftCap();
        emit Deposited(msg.sender, assetAmount, redeemableAmount, shares);
        return redeemableAmount;
    }

RageDnDepository.sol#L120 can withdraw a redeemable amount that should not be withdrawn

    function redeem(address asset, uint256 redeemableAmount)
        external
        onlyController
        returns (uint256)
    {
        if (asset != assetToken) {
            revert UnsupportedAsset(asset);
        }
        uint256 assetAmount = _redeemableToAssets(redeemableAmount);
        redeemableUnderManagement -= redeemableAmount;
        netAssetDeposits -= assetAmount;
        uint256 shares = vault.withdraw(
            assetAmount,
            address(controller),
            address(this)
        );
        emit Withdrawn(msg.sender, assetAmount, redeemableAmount, shares);
        return assetAmount;
    }

Code Snippet

    function redeem(address asset, uint256 redeemableAmount)
        external
        onlyController
        returns (uint256)
    {
        if (asset != assetToken) {
            revert UnsupportedAsset(asset);
        }
        uint256 assetAmount = _redeemableToAssets(redeemableAmount);
        redeemableUnderManagement -= redeemableAmount;
        netAssetDeposits -= assetAmount;
        uint256 shares = vault.withdraw(
            assetAmount,
            address(controller),
            address(this)
        );
        emit Withdrawn(msg.sender, assetAmount, redeemableAmount, shares);
        return assetAmount;
    }

    function deposit(address asset, uint256 assetAmount)
        external
        onlyController
        returns (uint256)
    {
        if (asset != assetToken) {
            revert UnsupportedAsset(asset);
        }
        netAssetDeposits += assetAmount;
        IERC20(assetToken).approve(address(vault), assetAmount);
        uint256 shares = vault.deposit(assetAmount, address(this));
        uint256 redeemableAmount = _assetsToRedeemable(assetAmount);
        redeemableUnderManagement += redeemableAmount;
        _checkSoftCap();
        emit Deposited(msg.sender, assetAmount, redeemableAmount, shares);
        return redeemableAmount;
    }

    function assetsDeposited() external view returns (uint256) {
        return netAssetDeposits;
    }

    /// @dev returns the shares currently owned by this depository
    function getDepositoryShares() external view returns (uint256) {
        return vault.balanceOf(address(this));
    }

    /// @dev returns the assets currently owned by this depository.
    function getDepositoryAssets() public view returns (uint256) {
        return vault.convertToAssets(vault.balanceOf(address(this)));
    }

Tool used

Manual Review

Recommendation

consider adding to control the permissions of a different user than the controller