sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

0xVolodya - users will receive not appropriate target tokens for their input - underlyingAmount #1

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

0xVolodya

medium

users will receive not appropriate target tokens for their input - underlyingAmount

Summary

According to the docs users should redeem/withdraw their principal tokens based on scale at maturity time and not on current scale like its happening in withdraw

Vulnerability Detail

    function withdraw(
        uint256 underlyingAmount,
        address to,
        address from
    ) external override nonReentrant expired returns (uint256) {
        GlobalScales memory _gscales = gscales;
        uint256 cscale = _updateGlobalScalesCache(_gscales);

        // Compute the shares to be redeemed
        uint256 sharesRedeem = underlyingAmount.divWadDown(cscale); // @audit current scale
...
    }

src/Tranche.sol#L337

Just like its happening in redeem

    function redeem(
        uint256 principalAmount,
        address to,
        address from
    ) external override nonReentrant expired returns (uint256) {
        GlobalScales memory _gscales = gscales;
        _updateGlobalScalesCache(_gscales);

        // Compute the shares to be redeemed
        uint256 shares = _computeSharesRedeemed(_gscales, principalAmount); // @audit scale at maturity time
...
    }

Impact

Users will burn a random(based on the current scale) amount of principal tokens and not the amount based on the underlying amount

Code Snippet

Tool used

Manual Review

Recommendation

    function withdraw(
        uint256 underlyingAmount,
        address to,
        address from
    ) external override nonReentrant expired returns (uint256) {
        GlobalScales memory _gscales = gscales;
        uint256 cscale = _updateGlobalScalesCache(_gscales);

        // Compute the shares to be redeemed
-        uint256 sharesRedeem = underlyingAmount.divWadDown(cscale);
+        uint256 sharesRedeem = underlyingAmount.divWadDown(_gscales);
        uint256 principalAmount = _computePrincipalTokenRedeemed(_gscales, sharesRedeem);

        // Update the global scales
        gscales = _gscales;
        // Burn PT tokens from `from`
        _burnFrom(from, principalAmount);
        // Withdraw underlying tokens from the adapter and transfer them to `to`
        _target.safeTransfer(address(adapter), sharesRedeem);
        (uint256 underlyingWithdrawn, ) = adapter.prefundedRedeem(to);

        emit Redeem(from, to, underlyingWithdrawn);
        return principalAmount;
    }
sherlock-admin commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: current scale should be used; medium(3)