search

sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

0x52 - GeneralRepay#repayJUSD returns excess USDC to `to` address rather than msg.sender #459

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

GeneralRepay#repayJUSD returns excess USDC to to address rather than msg.sender

Summary

When using GeneralRepay#repayJUSD to repay a position on JUSDBank, any excess tokens are sent to the to address. While this is fine for users that are repaying their own debt this is not good when repaying for another user. Additionally, specifying an excess to repay is basically a requirement when attempting to pay off the entire balance of an account. This combination of factors will make it very likely that funds will be refunded incorrectly.

Vulnerability Detail

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/flashloanImpl/GeneralRepay.sol#L65-L69

        IERC20(USDC).approve(jusdExchange, borrowBalance);
        IJUSDExchange(jusdExchange).buyJUSD(borrowBalance, address(this));
        IERC20(USDC).safeTransfer(to, USDCAmount - borrowBalance);
        JUSDAmount = borrowBalance;
    }

As seen above, when there is an excess amount of USDC, it is transferred to the to address which is the recipient of the repay. When to != msg.sender all excess will be sent to the recipient of the repay rather than being refunded to the caller.

Impact

Refund is sent to the wrong address if to != msg.sender

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/flashloanImpl/GeneralRepay.sol#L32-L73

Tool used

Manual Review

Recommendation

Either send the excess back to the caller or allow them to specify where the refund goes

JoscelynFarr commented 1 year ago

fix link: https://github.com/JOJOexchange/JUSDV1/commit/7382ce40dd54f0a396fb5d3f13ab3cfede0493e2

IAm0x52 commented 1 year ago

Fix looks good. Excess USDC is now refunded to msg.sender