Omniscia Echidna Finance Audit

PlatypusProxy Manual Review Findings

PlatypusProxy Manual Review Findings

PPY-01M: Overly Centralized Functionality

Description:

The linked function is immensely powerful and has no true necessity as being present in the code.

Example:

contracts/core/PlatypusProxy.sol
211/// @notice Execute arbitrary function calls
212/// @dev Only operator
213/// @param _to contract to call
214/// @param _value amount of avax
215/// @param _data calldata to pass to to
216function execute(
217    address _to,
218    uint256 _value,
219    bytes calldata _data
220) external returns (bool, bytes memory) {
221    require(msg.sender == operator, "!operator");
222

223    (bool success, bytes memory result) = _to.call{value: _value}(_data);
224

225    return (success, result);
226}

Recommendation:

We advise it to be removed from the codebase and replaced by more explicit workflows the contract is meant to support as it is upgradeable.

Alleviation:

The next prefixed entry is now cleared out in both referenced functions.

PPY-02M: Improper Approval Assignment

TypeSeverityLocation
Logical FaultMediumPlatypusProxy.sol:L208

Description:

The code improperly assigns the maximum approval possible to the caller as it invokes the deprecated safeApprove function that will fail in case the caller has a non-zero allowance already set.

Example:

contracts/core/PlatypusProxy.sol
203/// @notice Unlock the PTP
204/// @dev Can only be called by PTPDepositor when shutting down the system.
205function release() external override onlyDepositor {
206    uint256 amount = IVePtp(vePtp).users(address(this)).amount;
207    IVePtp(vePtp).withdraw(amount);
208    IERC20(ptp).safeApprove(msg.sender, type(uint256).max);
209}

Recommendation:

We advise the code to be replaced by a straight approve instruction similar to the optimization outlined in the separate finding.

Alleviation:

All commit instances were corrected, validating that the value to be written is non-zero and preventing duplicate executions.

PPY-03M: Deprecated Approval Paradigm

Description:

The linked statements utilize the safeApprove function that has been officially deprecated.

Example:

contracts/core/PlatypusProxy.sol
183/// @notice Transfer PTP from msg.sender and lock PTP into vePTP.
184/// @dev anyone can donate PTP :D
185/// @param amount amount of PTP to lock
186function donatePtp(uint256 amount) external override {
187    IERC20(ptp).safeTransferFrom(msg.sender, address(this), amount);
188    SafeERC20.safeApprove(IERC20(ptp), vePtp, 0);
189    SafeERC20.safeApprove(IERC20(ptp), vePtp, amount);
190

191    IVePtp(vePtp).deposit(amount);
192}

Recommendation:

We advise a direct approve instruction to be utilized instead as the statements are indeed meant to replace any previously set allowance to the new one.

Alleviation:

The ability for the code to execute arbitrary function calls was properly omitted from the codebase.

PPY-04M: Improper Commit Mechanism

Description:

The commit mechanism does not check whether the pending value has already been cleared permitting the owner to set the commit's sensitive value to 0 at all times.

Example:

contracts/core/PlatypusProxy.sol
61function commitBooster() external onlyOwner {
62    require(boosterDelay < block.timestamp);
63    boosters[nextBooster] = true;
64    nextBooster = address(0x0);
65}

Recommendation:

We advise an extra require check to be introduced ensuring the value-to-be-set is non-zero.

Alleviation:

The approval is now correctly set via an explicit approve instruction.

PPY-05M: Inexistent Clearance of Next Entry

Description:

The linked functions do not properly clean up the next prefixed entry as the commitBooster function does.

Example:

contracts/core/PlatypusProxy.sol
56function addBooster(address _booster) external onlyOwner {
57    nextBooster = _booster;
58    boosterDelay = block.timestamp + DELAY;
59}
60

61function commitBooster() external onlyOwner {
62    require(boosterDelay < block.timestamp);
63    boosters[nextBooster] = true;
64    nextBooster = address(0x0);
65}
66

67function removeBooster(address _booster) external onlyOwner {
68    boosters[_booster] = false;
69}
70

71function setDepositor(address _depositor) external onlyOwner {
72    nextDepositor = _depositor;
73    depositorDelay = block.timestamp + DELAY;
74}
75

76function commitDepositor() external onlyOwner {
77    require(depositorDelay < block.timestamp);
78    depositor = nextDepositor;
79}
80

81function setOperator(address _operator) external onlyOwner {
82    nextOperator = _operator;
83    operatorDelay = block.timestamp + DELAY;
84}
85

86function commitOperator() external onlyOwner {
87    require(operatorDelay < block.timestamp);
88    operator = nextOperator;
89}

Recommendation:

We advise the next entries to be properly cleaned up by assigning the value of zero to them.

Alleviation:

A direct approve statement is now utilized in all referenced instances in place of the deprecated paradigm.

Prev

MerkleDistributor.sol (MDR-M)

Next

PtpDepositor.sol (PDR-M)

On this page