ElasticSwap contest - defsec's results

Handle

defsec

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deploy time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept

Revert strings > 32 bytes are here:

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L179

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L137

Tools Used

Manual Review

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes. That will affect gas optimization.

Handle

defsec

Vulnerability details

Impact

Gas overspending due to excessive external function calls

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L176

2. The following code is using totalSupply external call multiple times.

        require(this.totalSupply() > 0, "Exchange: INSUFFICIENT_LIQUIDITY");
        require(
            _baseTokenQtyMin > 0 && _quoteTokenQtyMin > 0,
            "Exchange: MINS_MUST_BE_GREATER_THAN_ZERO"
        );

        uint256 baseTokenReserveQty =
            IERC20(baseToken).balanceOf(address(this));
        uint256 quoteTokenReserveQty =
            IERC20(quoteToken).balanceOf(address(this));

        uint256 totalSupplyOfLiquidityTokens = this.totalSupply();

Tools Used

Code Review

Recommended Mitigation Steps

Store external interaction into the variable at the beginning of the function.

        uint256 totalSupplyOfLiquidityTokens = this.totalSupply();

        require(totalSupplyOfLiquidityTokens != 0, "Exchange: INSUFFICIENT_LIQUIDITY");
        require(
            _baseTokenQtyMin > 0 && _quoteTokenQtyMin > 0,
            "Exchange: MINS_MUST_BE_GREATER_THAN_ZERO"
        );

        uint256 baseTokenReserveQty =
            IERC20(baseToken).balanceOf(address(this));
        uint256 quoteTokenReserveQty =
            IERC20(quoteToken).balanceOf(address(this));

Handle

defsec

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

1. Navigate to the following contracts.

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L178

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L113

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L176

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L178

Tools Used

Code Review

Recommended Mitigation Steps

Use "!=0" instead of ">0" for the gas optimization.

Handle

defsec

Vulnerability details

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/libraries/MathLib.sol#L705

Tools Used

None

Recommended Mitigation Steps

Consider applying unchecked arithmetic where overflow/underflow is not possible.