sherlock-admin3
commented
2 months ago 1 comment(s) were left on this issue during the judging contest.
0xmystery commented:
Simple contracts with one of the parent contract not implementing storage gaps are considered low/informational
ZeroTrust
Medium
Corruptible Upgradability Pattern
Summary
Storage of PendlePTKelpVault and PendlePTStakedUSDeVault contracts might be corrupted during upgrade
Vulnerability Detail
The PendlePT staking vaults are meant to be upgradeable. However, they inherit contracts that are not upgrade-safe.
The BaseStrategyVault have gap storage, But The WithdrawRequestBase, BaseStakingVault and PendlePrincipalToken, which are inherited by PendlePT staking vaults .
Thus, adding new storage variables to WithdrawRequestBase, BaseStakingVault and PendlePrincipalToken can potentially overwrite the beginning of the storage layout of the child contracts, causing critical misbehavior in the system.
Impact
Storage of PendlePTKelpVault and PendlePTStakedUSDeVault contracts might be corrupted during upgrade, thus causing the vaults to be broken and assets to be stuck.
Code Snippet
https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/common/BaseStrategyVault.sol#L245
https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/common/WithdrawRequestBase.sol#L237
https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/BaseStakingVault.sol#L269
https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/PendlePrincipalToken.sol#L190
Tool used
Manual Review
Recommendation
Consider defining an appropriate storage gap in the upgradeable parent contracts at the end of all the storage variable definitions as follows: