Shell Protocol - pontifex's results

Lines of code

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

Vulnerability details

Impact

Lack of balance checks in the withdrawGivenOutputAmount function can lead to various unpredictable consequences due to the breach of requirements "The pool's balances of the x reserve and y reserve tokens must be greater than the MIN_BALANCE" and "The pool's ratio of y to x must be within the interval [MIN_M, MAX_M)".

Proof of Concept

The withdrawGivenOutputAmount receive an withdrawnAmount parameter which describes how many reserve tokens the user wants to withdraw. There are no balance checks in the withdrawGivenOutputAmount function and in the _reserveTokenSpecified function, similar with checks in the _swap function. So users can withdraw more tokens than is safety for the contract.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest using the _checkBalances function at the withdrawGivenOutputAmount function or at the _reserveTokenSpecified function.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Lack of balance checks in the depositGivenInputAmount function can lead to various unpredictable consequences due to the breach of requirement "The pool's ratio of y to x must be within the interval [MIN_M, MAX_M)".

Proof of Concept

The depositGivenInputAmount receive an depositedAmount parameter which describes how many reserve tokens the user wants to deposit. There are no balance checks in the depositGivenInputAmount function and in the _reserveTokenSpecified function, similar with checks in the _swap function. So users can deposit such an amount of tokens which breach the pool's ratio.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest using the _checkBalances function at the depositGivenInputAmount function or at the _reserveTokenSpecified function.

Assessed type

Other

L-1 Wrong comments

Mistakes in comments can be misleading. The comment amount cannot be less than 0 should be amount should be less than 0. There are 3 instances:

295:        // amount cannot be less than 0
296:        require(result < 0);


450:        // amount cannot be less than 0
451:        require(result < 0);


487:        // amount cannot be less than 0
488:        require(result < 0);

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

The comment We use FEE_UP because we want to increase the perceived amount of should be We use FEE_DOWN because we want to increase the perceived amount of.

459:     * @dev We use FEE_UP because we want to increase the perceived amount of
...
481:            FEE_DOWN,

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

The comment Calculates the b variable in the curve eq which is basically a sq. root of the inverse of x instantaneous price should be Calculates the b variable in the curve eq which is basically a sq. root of the of x instantaneous price.

120:       @notice Calculates the b variable in the curve eq which is basically a sq. root of the inverse of x instantaneous price
...
124:        return p_min(self).sqrt();

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

L-2 Insufficient comments for magic values.

It is hard to check constant values correctness due to insufficient comments.

157:    int128 constant MAX_M = 0x5f5e1000000000000000000;
163:    int128 constant MIN_M = 0x00000000000002af31dc461;
169:    int256 constant MAX_PRICE_VALUE = 1844674407370955161600000000;
175:    int256 constant MIN_PRICE_VALUE = 184467440737;

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

GAS-1 Use caching variable for t(self) in _addShares function

Use caching variables for t(self) storage variables in the p_min and p_max functions to prevent multiple storage reading.

98:        if (t(self) > ABDK_ONE) return self.px_final;
99:        else return self.px_init.mul(ABDK_ONE.sub(t(self))).add(self.px_final.mul(t(self)));

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

107:        if (t(self) > ABDK_ONE) return self.py_final;
108:        else return self.py_init.mul(ABDK_ONE.sub(t(self))).add(self.py_final.mul(t(self)));

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