Use contracts-upgradeable instead of contract variants of OpenZeppelin

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L13 https://github.com/code-423n4/2023-11-kelp/blob/main/src/NodeDelegator.sol#L12 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTOracle.sol#L14

Vulnerability details

Impact

OpenZeppelin’s contracts variants when used with upgradeability will result in negative impact on the overall contract functionality.

Check this OpenZeppelin warning about mixing contract variants with upgradeable-contract.

Proof of Concept

Upgradeable versions of contract i.e., LRTDepositPool, NodeDelegator and LRTOracle are using contract variants instead of contract-upgradeable variants that affect the contract working due to dependence on initialize functionality.

File: LRTDepositPool.sol
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

File: NodeDelegator..sol
import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol";
import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

File: LRTOracle.sol
import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol";
import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

When contracts are deployed with upgradeability they must consider the Upgradeable variant of OpenZeppelin rather than the simple contract variants as defined in OZ’s official docs.

Tools Used

Manual Review

Recommended Mitigation Steps

Import from contracts-upgradeable, not from contracts when any upgradeable OZ's contract is used.

Assessed type

Library

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Caught by other bots as NC or L. QA at best.

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid