[Audit Report] Trader Joe v2 (1)

https://code4rena.com/reports/2022-10-traderjoe#scope

Code4rena

트레이더 조 v2 의 오딧 리포트를 살펴본다.

 

보기 전에 트레이더 조의 프로토콜 분석 리포트 

https://research.despread.io/reports-kr-traderjoe/

 

길지만 정리를 상당히 잘해놔서 이해가 잘된다. 그래도 어려운 건 맞음.ㅜ 

그래도 TVL은 아비트럼의 다른 프로토콜보다 낮을지라도 수익은 상당히 높은 축에 낀다. 

 

https://github.com/code-423n4/2022-10-traderjoe

contest details

 

[H-01] Transfering funds to yourself increase your balance

LBToken.sol 에서 safeTransferFrom함수를 보면 안에 내부함수로 _transfer를 사용하는 것을 볼 수 있는데 거기서 중복의 문제가 생긴다는 것이다. 

    function safeTransferFrom(
        address _from,
        address _to,
        uint256 _id,
        uint256 _amount
    ) public virtual override checkAddresses(_from, _to) checkApproval(_from, msg.sender) {
        address _spender = msg.sender;

        _transfer(_from, _to, _id, _amount);

        emit TransferSingle(_spender, _from, _to, _id, _amount);
    }

 

    function _transfer(
        address _from,
        address _to,
        uint256 _id,
        uint256 _amount
    ) internal virtual {
        uint256 _fromBalance = _balances[_id][_from];
        if (_fromBalance < _amount) revert LBToken__TransferExceedsBalance(_from, _id, _amount);

        _beforeTokenTransfer(_from, _to, _id, _amount);

        uint256 _toBalance = _balances[_id][_to];

        unchecked {
            _balances[_id][_from] = _fromBalance - _amount;
            _balances[_id][_to] = _toBalance + _amount;
        }

        _remove(_from, _id, _fromBalance, _amount);
        _add(_to, _id, _toBalance, _amount);
    }

그런데 이런 방식은 OZ의 ERC20에서 사용하는 방식이랑 동일하다. 

 

그래서 _transfer부분을

unchecked {
    if (_from != _to) {
        _balances[_id][_from] = _fromBalance - _amount;
        _balances[_id][_to] = _toBalance + _amount;
    }
}

from하고 to 하고 다르게 해서 하라는 것이다. 

 

그렇다면 ERC20과 LBToken은 어디가 다르게 from과 to 같으면 안 된다는 것일까. 

contest details을 보면 컨트랙트에 대한 설명이 나와있다. 

https://github.com/code-423n4/2022-10-traderjoe

 

 

[H-02] Inccorrect output amount calculation for Trader Joe V1 pools

Output 수량이 Trader Joe V1 pool에서 잘 못 계산된다. 멀티플 풀을 통해서 토큰을 스왑 하거나 풀의 일부가 V1일 때. 

계산된 수량이 예상된 것 보다 항상 적을 것이다. V1 pool이 포함되어 있다면. 

 

LBRouter는 유저가 상호작용하는 상위레벨의 컨트랙트다. 이 컨트랜트는 많은 보안적 체크를 실행하고 유저 친화적으로 LBPair컨트랙트를 사용하도록 돕는다. 

- swapExactTokensForTokensSupportingFeeOnTransferTokens

- swapExactTokensForAVAXSupportingFeeOnTransferTokens

- swapExactAVAXForTokensSupportingFeeOnTrnasferTokens

이 3개의 함수들은 _swapSupportingFeeOnTransferTokens 함수를 사용한다. 이 함수는 사실상 스왑을 실행한다. 

이 함수는 Trader Joe V1, V2 pool을 서포트한다. _bitStep이 0일 때( V2 pool에서는 절대 ture가 아니다 ), 현재 풀은 V1풀이라고 가정해 보자.  V1 pool에서 그 함수는 output 수량을 계산한다. pool에 남아있는 것과 balance를 통해서. ( 밑에 사진 ) 

 그러나 이 계산은 잘못되었다.

 

                     if (_token < _tokenNext) {
                         uint256 _balance = _token.balanceOf(_pair);
-                        uint256 _amountOut = (_reserve1 * (_balance - _reserve0) * 997) / (_balance * 1_000);
+                        uint256 amountInWithFee = (_balance - _reserve0) * 997;
+                        uint256 _amountOut = (_reserve1 * amountInWithFee) / (_reserve0 * 1_000 + amountInWithFee);
                         IJoePair(_pair).swap(0, _amountOut, _recipient, "");
                     } else {
                         uint256 _balance = _token.balanceOf(_pair);
-                        uint256 _amountOut = (_reserve0 * (_balance - _reserve1) * 997) / (_balance * 1_000);
+                        uint256 amountInWithFee = (_balance - _reserve1) * 997;
+                        uint256 _amountOut = (_reserve0 * amountInWithFee) / (_reserve1 * 1_000 + amountInWithFee);
                         IJoePair(_pair).swap(_amountOut, 0, _recipient, "");
                     }

위와 같이 수정을 해야한다. JoeLibrary에 있는 getAmountOut함수처럼 해야 한다. 

 

swapExactTokensForTokensSupportingFeeOnTransferTokens 함수를 사용해서 나온 값과 

getAmountOut 함수를 나온 값을 비교해서 검증을 하고 있다. 

        router.swapExactTokensForTokensSupportingFeeOnTransferTokens(
            amountIn, 0, steps, path, address(this), block.timestamp + 1000
        );
        // This amount was calculated incorrectly.
        assertEq(actualAmountOut, 987030000000000000); // Equals to 989970211528238869 when fixed.
        address _pair = address(this);
        uint256 expectedAmountOut;
        // Reproduce the calculations using JoeLibrary.getAmountIn. This piece:
        
        (uint256 _reserve0, uint256 _reserve1, ) = IJoePair(_pair).getReserves();
        if (address(token) < address(this)) {
            uint256 _balance = token.balanceOf(_pair);
            expectedAmountOut = JoeLibrary.getAmountOut(_balance - _reserve0, _reserve0, _reserve1);
        } else {
            uint256 _balance = token.balanceOf(_pair);
            expectedAmountOut = JoeLibrary.getAmountOut(_balance - _reserve1, _reserve1, _reserve0);
        }
        // This is the correct amount.
        assertEq(expectedAmountOut, 989970211528238869);
        // The wrong amount is smaller than the expected one.
        assertEq(expectedAmountOut - actualAmountOut, 2940211528238869);