Platform: Code4rena
Start Date: 21/11/2022
Pot Size: $90,500 USDC
Total HM: 18
Participants: 101
Period: 7 days
Judge: Picodes
Total Solo HM: 4
Id: 183
League: ETH
Rank: 42/101
Findings: 2
Award: $93.14
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xSmartContract
Also found by: 0xAgro, 0xNazgul, 0xPanda, 0xbepresent, 0xfuje, Awesome, B2, Bnke0x0, Deivitto, Diana, Funen, Jeiwan, JohnSmith, Josiah, R2, RaymondFam, Rolezn, Sathish9098, Waze, adriro, aphak5010, brgltd, btk, carrotsmuggler, ch0bu, chaduke, codeislight, codexploder, cryptostellar5, csanuragjain, danyams, datapunk, delfin454000, deliriusz, eierina, erictee, fatherOfBlocks, gz627, gzeon, hansfriese, hihen, jadezti, joestakey, keccak123, martin, nameruse, oyc_109, pedr02b2, perseverancesuccess, rbserver, rotcivegaf, rvierdiiev, sakshamguruji, shark, simon135, subtle77, unforgiven, xiaoming90, yixxas
53.4851 USDC - $53.49
zero address check in constructorMissing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.
PxERC20(_pirexRewards, "Pirex GMX", "pxGMX", 18)
File: /src/PxGmx.sol (line 10)
require() should be used instead of assert()assert(feeAmount + postFeeAmount == assets);
File: src/PirexGmx.sol (line 225)
__gap[50] storage variable to allow for new storage variables in later versionsWhile some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
contract AutoPxGmx is ReentrancyGuard, Owned, PirexERC4626 {
File: src/vaults/AutoPxGmx.sol (line 14)
contract AutoPxGlp is PirexERC4626, PxGmxReward, ReentrancyGuard {
File: src/vaults/AutoPxGlp.sol (line 14)
Other instances of this issue are:
block.timestampBlock timestamps have historically been used for a variety of applications, such as entropy for random numbers, locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.
(block.timestamp - globalState.lastUpdate) *
File: src/vaults/PxGmxReward.sol (line 54)](https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PxGmxReward.sol#L54)
globalState.lastUpdate = block.timestamp.safeCastTo32();
File: src/vaults/PxGmxReward.sol (line 54)](https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PxGmxReward.sol#L57)
Other instances of this issue are:
INITIALIZE functions can be front runThis function can be called by everyone. So an attacker may watch yhe mempool for this function and may frontrun it by supplying more gas. This may result in unintended behaviour
function initialize() public initializer {
File: src/PirexRewards.sol (line 85)
indexed fieldsEach event should use three indexed fields if there are three or more fields.
event SetTreasuryFeePercent(uint8 _treasuryFeePercent); event DistributeFees( ERC20 indexed token, uint256 distribution, uint256 treasuryDistribution, uint256 contributorsDistribution
File: src/PirexFees.sol (line 36-40)
event GlobalAccrue(uint256 lastUpdate, uint256 lastSupply, uint256 rewards);
File: src/vaults/PxGmxReward.sol (line 21)
Other instances of this issue are:
NatSpec is incomplete/// @audit Missing: `@return` /** @notice Override transfer method to allow for pre-transfer internal hook @param to address Account receiving apxGLP @param amount uint256 Amount of apxGLP */ function transfer(address to, uint256 amount)
File: src/vaults/PirexERC4626.sol (line 233-238)
/// @audit Missing: '@param` /** @notice Override the withdrawal method to make sure compound is called before withdrawing */ function withdraw(
File: src/vaults/AutoPxGlp.sol (line 433-436)
Other instances of this issue are:
@param & @return@param@returnNatSpec// SPDX-License-Identifier: MIT
File: src/interfaces/IAutoPxGlp.sol (line 0)
// SPDX-License-Identifier: MIT
File: src/interfaces/IProducer.sol (line 0)
// SPDX-License-Identifier: MIT
File: src/interfaces/IPirexRewards.sol (line 0)
garbage value in mapping for deleting thatIf there is a mapping data structure present inside struct, then deleting the struct doesn't delete the mapping. Instead one should use lock to lock that data structure from further use.
delete producerTokens[producerToken].rewardRecipients[msg.sender][
File: src/PirexRewards.sol (line 139)
delete producerTokens[producerToken].rewardRecipients[lpContract][
File: src/PirexRewards.sol (line 470)
The source files have different solidity compiler ranges referenced. This leads to potential security flaws between deployed contracts depending on the compiler version chosen for any particular file. It also greatly increases the cost of maintenance as different compiler versions have different semantics and behavior.
File: src/interfaces/IPirexRewards.sol (line 2) uses
pragma solidity >=0.8.0;whereas rest of the contracts usespragma solidity 0.8.17;
#0 - c4-judge
2022-12-05T08:52:13Z
Picodes marked the issue as grade-b
🌟 Selected for report: gzeon
Also found by: 0xPanda, 0xSmartContract, B2, Deivitto, Diana, JohnSmith, PaludoX0, Rahoz, RaymondFam, ReyAdmirado, Rolezn, Schlagatron, Secureverse, Tomio, __141345__, adriro, ajtra, aphak5010, c3phas, chaduke, codeislight, cryptonue, datapunk, dharma09, halden, karanctf, keccak123, oyc_109, pavankv, sakshamguruji, saneryee, unforgiven
39.6537 USDC - $39.65
returnsUsing both named returns and a return statement isn’t necessary. Removing one of those can improve code clarity:
return (userState.lastUpdate, userState.lastBalance, userState.rewards);
File: src/PirexRewards.sol (line 220)
address mappings can be combined into a single mapping of address to a struct, where appropriate.Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot
22 mapping(address => UserState) userStates; 24 mapping(address => mapping(ERC20 => address)) rewardRecipients;
23 mapping(ERC20 => uint256) rewardStates; 31 mapping(ERC20 => ProducerToken) public producerTokens;
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
function _globalAccrue() internal {
File: src/vaults/PxGmxReward.sol (line 49)
function _userAccrue(address user) internal {
File: src/vaults/PxGmxReward.sol (line 68)
Other instances of this issue are
<x> += <y> costs more gas than <x> = <x> + <y> for state variablesrewardState += rewardAmount;
File: src/vaults/PxGmxReward.sol (line 95)
emit GlobalAccrue(block.timestamp, totalSupply, rewards);
File: src/vaults/PxGmxReward.sol (line 61)
emit UserAccrue(user, block.timestamp, balance, rewards);
File: src/vaults/PxGmxReward.sol (line 83)
REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLEIf a function modifier such as onlyOwneris used, the function will revert if a normal user tries to pay the function. Marking the function as payablewill lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
function setVoteDelegate(address voteDelegate) external onlyOwner {
File: src/PirexGmx.sol (line 884)
function initiateMigration(address newContract)
File: src/PirexGmx.sol (line 921)
Other instances of this issue are
#0 - c4-judge
2022-12-05T11:11:04Z
Picodes marked the issue as grade-b