code-423n4 / 2023-08-shell-findings

3 stars 2 forks source link

Swap Tokens with Fee Consideration and Balance Checks #180

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L549 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L552

Vulnerability details

Impact

Detailed description of the impact of this finding.

In the _swap() function, the discrepancy lies in the usage of the variable specifiedAmount instead of roundedSpecifiedAmount when checking the final balance in the _swap() function.

        if (specifiedToken == SpecifiedToken.X) {
            computedAmount = _applyFeeByRounding(yf - yi, feeDirection);
            _checkBalances(xi + specifiedAmount, yi + computedAmount);
        } else {
            computedAmount = _applyFeeByRounding(xf - xi, feeDirection);
            _checkBalances(xi + computedAmount, yi + specifiedAmount);
        }

In the code snippet, when specifiedToken == SpecifiedToken.X, the correct calculation for the final X balance should be xi + roundedSpecifiedAmount. However, the code uses xi + specifiedAmount instead. Similarly, if specifiedToken == SpecifiedToken.Y, it uses yi + specifiedAmount.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

Recommend to use roundedSpecifiedAmount instead of specifiedAmount in _checkBalances() call. For example:

        if (specifiedToken == SpecifiedToken.X) {
            computedAmount = _applyFeeByRounding(yf - yi, feeDirection);
            _checkBalances(xi + roundedSpecifiedAmount, yi + computedAmount);
        } else {
            computedAmount = _applyFeeByRounding(xf - xi, feeDirection);
            _checkBalances(xi + computedAmount, yi + roundedSpecifiedAmount);
        }

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as primary issue

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as low quality report

0xRobocop commented 1 year ago

Invalid

specifiedAmount is correct.

c4-judge commented 1 year ago

JustDravee marked the issue as unsatisfactory: Invalid