SteerPeriphery Manual Review Findings

SPY-01M: Inexistent Initialization of Base Implementation

Type	Severity	Location
Language Specific		SteerPeriphery.sol:L23-L28

Description:

The contract does not properly initialize the base logic implementation permitting it to be taken over by a malicious party.

Impact:

While not an active security threat, it can evolve into one if any form of delegatecall capability is introduced in one of the dependencies of the contract that could cause it to invoke a selfdestruct instruction.

Example:

Copy
23contract SteerPeriphery is
24    ISteerPeriphery,
25    Initializable,
26    OwnableUpgradeable,
27    UUPSUpgradeable
28{

Recommendation:

We advise a constructor to be introduced that simply invokes the initializer modifier to ensure that the logic implementation cannot be initialized maliciously.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

A constructor was introduced that properly invokes the initializer modifier and disallows initialization of the logic implementation, alleviating this exhibit in full.

SPY-02M: Inexistent Clearance of Approvals

Type	Severity	Location
Language Specific		SteerPeriphery.sol:L147-L148, L150-L156

Description:

The vault deposit system is incompatible with a subset of EIP-20 tokens (such as Tether / USDT) due to the way it performs approvals towards the vault address.

Impact:

The Steer Protocol team stated that they wish to intend all EIP-20 tokens including Tether and as such the vulnerability is applicable to their system.

Example:

Copy
130function deposit(
131    address _vaultAddress,
132    uint256 amount0Desired,
133    uint256 amount1Desired,
134    uint256 amount0Min,
135    uint256 amount1Min,
136    address to
137) external {
138    IMultiPositionManager vaultInstance = IMultiPositionManager(
139        _vaultAddress
140    );
141    IERC20Metadata token0 = IERC20Metadata(vaultInstance.token0());
142    IERC20Metadata token1 = IERC20Metadata(vaultInstance.token1());
143    if (amount0Desired > 0)
144        token0.transferFrom(msg.sender, address(this), amount0Desired);
145    if (amount1Desired > 0)
146        token1.transferFrom(msg.sender, address(this), amount1Desired);
147    token0.approve(_vaultAddress, amount0Desired);
148    token1.approve(_vaultAddress, amount1Desired);
149
150    (, uint256 amount0, uint256 amount1) = vaultInstance.deposit(
151        amount0Desired,
152        amount1Desired,
153        amount0Min,
154        amount1Min,
155        to
156    );
157
158    if (amount0Desired > amount0) {
159        token0.transfer(msg.sender, amount0Desired - amount0);
160    }
161    if (amount1Desired > amount1) {
162        token1.transfer(msg.sender, amount1Desired - amount1);
163    }
164}

Recommendation:

We advise any approval set to the _vaultAddress to be cleared after the deposit has been performed as tokens such as USDT / Tether do not allow consecutive non-zero approve instructions.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

Approvals are now adequately cleared in the respective if clauses that validate whether the full amount (and thus approval) was consumed, alleviating this exhibit in full.