Poolshark Vaults Security Audit

Audit Report Revisions

Audit Overview

We were tasked with performing an audit of the Steer Protocol codebase and in particular their Poolshark Vaults module, a fee structure update, as well as two staking implementations.

The staking implementations appear to be similar to Synthetix's original staking system with minor modifications, such as support for two parallel reward tokens.

We observed a flaw in the factories of the staking implementations that permit a skewed distribution of rewards.

In relation to the AlgebraLiquidityManagers and UniLiquidityManager folders, an upgrade of existing implementations to a new identifier-based fee system was introduced.

We observed a significant flaw in the migration process of previously-deployed instances which would result in inoperable migrations. Additionally, we observed shifts in the storage structure of the contracts that would result in data corruption.

Finally, the PoolSharkLiquidityManagers folder was evaluated from an integration perspective to the Poolshark AMM system.

The contracts utilized an updated pragma version which resulted in a discrepancy within the code's functional behaviour due to the implicit usage of checked arithmetic.

Additionally, a significant discrepancy in the Poolshark project's own documentation was observed; a burn of 0 is specified as yielding the fees accumulated within a position per their documentation but the code indicates a burn of 0 would result in fees being compounded instead.

The Steer Protocol team was aware of this fact and thus issues range burn operations of 1e-38 to capture any fees the position has accumulated. While this is slightly inefficient and dilutes shareholders of the liquidity provider, the dilution is miniscule and thus should not result in observable discrepancies.

Three potential denial-of-service issues were detected in the Poolshark integration that result from Poolshark's unique EIP-1155 position model.

Some static analysis results, such as require checks with no explicit error message, have been filtered out of the report due to the contracts approaching the EVM's bytecode limitation.

As a final note, we advise constant declarations that are shared between multiple implementations to instead be relocated to a single file, greatly easing maintainability as well as increasing the code quality of the codebase.

We advise the Steer Protocol team to closely evaluate all minor-and-above findings identified in the report and promptly remediate them as well as consider all optimizational exhibits identified in the report.

Post-Audit Conclusion

The Steer Protocol team iterated through all findings within the report and provided us with a revised commit hash to evaluate all exhibits on.

We evaluated all alleviations performed by Steer Protocol and have identified that most exhibits have either been acknowledged or properly alleviated.

The referenced exhibits that follow have either been fully alleviated albeit inefficiently, or have not been properly considered and should be revisited by the Steer Protocol team: FMR-02M, FMR-03C, PSB-01C

Multiple updates were performed in the latest iteration that have an impact on the overall upgrade flow of the system from the original commit hash to the latest commit hash.

The ramifications of the latest changes need to be revisited, in addition to the following two findings being validated in depth to be securely acknowledged: DSG-01M, SSG-01M

As a result, the current iteration of the audit report is not final and a follow-up version will be produced once the overall upgrade and migration flow has been verified with the latest implementation in mind.

Post-Audit Conclusion (dc34c6a066)

The Steer Protocol team provided us with a follow-up commit to evaluate the addressment of FMR-03C as well as certain changes applied in relation to a distinct audit we prepared for their team.

We validated that FMR-03C has been properly addressed, we incorporated the follow-up response of the Steer Protocol team to the FMR-02M exhibit's contents, and we performed a re-evaluation of the StakingSingleRewards and StakingDualRewards contracts in light of the reward and staking token equivalency requirement.

We identified two flaws in each that would arise from such a configuration, and these concerns have been raised in the relevant exhibits: DSG-01M, SSG-01M

Post-Audit Conclusion (3607cb04d9)

This chapter contains certain misconceptions as to how the storage updates were performed, and the next Post-Audit Conclusion chapter should be visited instead.

The Steer Protocol team proceeded with alleviating the misbehaviours described in the SSG-01M and DSG-01M exhibits, ensuring that the staking implementations behave as expected when the reward token is also a staked one.

Additionally, our team was supplied with the commit hash of the beta branch of the repository from which the upgrade of the contracts is expected to be performed from; namely 002221b4d5d65bd7692d1f335f10b18a58c71278.

We evaluated the delta between the implementations, and have identified that the storage structures are incompatible for the following contracts:

• contracts/vault-types/AlgebraLiquidityManagers/AlgebraMultiPositionLiquidityManager.sol
• contracts/vault-types/QuickSwapLiquidityManagers/QuickSwapMultiPositionLiquidityManager.sol
• contracts/vault-types/QuickSwapLiquidityManagers/QuickSwapSinglePositionLiquidityManager.sol
• contracts/vault-types/UniLiquidityManager/MultiPositionLiquidityManager.sol

Commented-out implementations were not considered in this evaluation, and the Integral related contracts are expected to be fresh deployments.

The incompatibility of the aforementioned contracts stems from their Base dependencies which have had storage variables introduced. As storage is linearly flattened in Solidity, the positional data in each Multi and Single contract implementation would be shifted to an empty data slot.

Illustrated via example:

Copy
// This is the initial implementation 
contract A {
    uint256 public dataPoint1 = 1;
}

contract B is A {
    uint256 public dataPoint2 = 2;
}

/**
 * The above will be translated to the following sequence:
 * - First Storage Slot: dataPoint1
 * - Second Storage Slot: dataPoint2
 */

/*======================================*/

// Let us consider the following upgrade
contract A {
    uint256 public dataPoint1;
    uint256 public dataPoint3;
}

contract B is A {
    uint256 public dataPoint2;
}

/**
 * The above will be translated to the following sequence:
 * - First Storage Slot: dataPoint1
 * - Second Storage Slot: dataPoint3
 * - Third Storage Slot: dataPoint2
 *
 * As such, when `B` is upgraded, the following conditions will be true:
 * - `A.dataPoint1 == 1`
 * - `A.dataPoint3 == 2`
 * - `A.dataPoint2 == 0`
 */

As can be observed above, an introduction of dataPoint3 in the A implementation (i.e. Base) will shift the data locations of the child B implementation (i.e. Multi & Single).

We advise regression testing to be introduced as well as fork-based testing for any upgrades of the Steer Protocol. Furthermore, we advise the code updates performed to the codebase to be re-assessed and made compatible with the existing storage structure of the deployed contracts as otherwise significant risk of data loss will be present.

Updated Post-Audit Conclusion (3607cb04d9)

The Steer Protocol evaluated our feedback, and clarified that the way they counteracted the storage slot shifting we describe in the previous chapter is via the _gap variables and an update of their length.

In light of this clarification, we re-assessed the delta of the storage updates and confirmed that the storage structures are sound and are properly compatible between versions.

Additionally, the Steer Protocol team confirmed that regression testing via forks is being conducted prior to the deployment of each update to ensure that storage compatibility is not breached.

Given that all findings described within the audit report have been properly alleviated and the storage-related concerns we raised were unfounded, we consider all outputs of the audit report properly consumed by the Steer Protocol team with no further actions expected.

Audit Synopsis

Severity	Identified	Alleviated	Partially Alleviated	Acknowledged
	5	5	0	0
	26	24	0	2
	8	8	0	0
	4	4	0	0
	5	5	0	0

During the audit, we filtered and validated a total of 14 findings utilizing static analysis tools as well as identified a total of 34 findings during the manual review of the codebase. We strongly recommend that any minor severity or higher findings are dealt with promptly prior to the project's launch as they can introduce potential misbehaviours of the system as well as exploits.

Total Alleviations

The list below covers each segment of the audit in depth and links to the respective chapter of the report:

• 🎯 Scope
• 💻 Compilation
• 🔍 Static Analysis
• 👁️ Manual Review
• 🖋️ Code Style