Platform: Code4rena
Start Date: 22/05/2024
Pot Size: $20,000 USDC
Total HM: 6
Participants: 126
Period: 5 days
Judge: 0xsomeone
Total Solo HM: 1
Id: 379
League: ETH
Rank: 113/126
Findings: 1
Award: $0.01
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: robertodf99
Also found by: 0xAadi, 0xAkira, 0xdice91, 0xhacksmithh, 0xleadwizard, AgileJune, Bauchibred, Bbash, Beosin, Bigsam, Dots, EPSec, EaglesSecurity, Eeyore, Evo, John_Femi, Mahmud, MrPotatoMagic, RotiTelur, Rushkov_Boyan, Sabit, Sentryx, Stormreckson, Topmark, Tychai0s, Utsav, Walter, ZanyBonzy, ZdravkoHr, adam-idarrha, araj, aslanbek, avoloder, bigtone, brevis, brgltd, carrotsmuggler, crypticdefense, dd0x7e8, dhank, djanerch, falconhoof, iamandreiski, joaovwfreire, leegh, merlinboii, mitko1111, pamprikrumplikas, pfapostol, prapandey031, swizz, trachev, twcctop, typicalHuman, unique, xyz
0.0148 USDC - $0.01
https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L177
The approveUSDPrice function does not check if a user has already disapproved a proposal before approving it. This oversight allows users with the appropriate role to disapprove and then approve the same proposal, which is a security concern.
The approveUSDPrice function currently only checks if a user has approved the proposal but does not verify if they have disapproved it. As a result, users who have the role can disapprove a proposal and then approve it afterward. This loophole undermines the integrity of the approval process within the contract.
Manual Review
Add a condition to check if the user has disapproved the proposal before allowing them to approve it. Here is the suggested code modification:
+ if (usdUpdateProposal.disapprovals[msg.sender] == _usdProposalId) { + revert ProposalAlreadyDisapprovedError(); + }
This change ensures that users who have disapproved a proposal cannot approve it subsequently, maintaining the intended integrity of the contract's proposal approval process.
Governance
#0 - c4-judge
2024-06-05T12:42:43Z
alex-ppg marked the issue as satisfactory