Platform: Code4rena
Start Date: 25/01/2023
Pot Size: $36,500 USDC
Total HM: 11
Participants: 173
Period: 5 days
Judge: kirk-baird
Total Solo HM: 1
Id: 208
League: ETH
Rank: 144/173
Findings: 2
Award: $7.80
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: adriro
Also found by: 0xRobocop, 0xmrhoodie, 0xngndev, AkshaySrivastav, ArmedGoose, Atarpara, Bauer, CodingNameKiki, ElKu, Garrett, HollaDieWaldfee, IllIllI, Iurii3, KIntern_NA, KmanOfficial, Lotus, M4TZ1P, MiniGlome, Ruhum, SovaSlava, bin2chen, bytes032, carrotsmuggler, cccz, chaduke, codeislight, cryptonue, doublesharp, evan, fs0c, glcanvas, gzeon, hansfriese, hihen, hl_, holme, horsefacts, ladboy233, lukris02, mahdikarimi, manikantanynala97, martin, mert_eren, mrpathfindr, omis, peakbolt, peanuts, prestoncodes, rbserver, rvierdiiev, sashik_eth, timongty, tnevler, trustindistrust, usmannk, wait, yixxas, zadaru13, zaskoh
0.7512 USDC - $0.75
Judge has assessed an item in Issue #664 as 3 risk. The relevant finding follows:
[L-1] ERC20 Quest: withdrawFee() function should only be able to be called once instead of multiple times
Issue: The withdrawFee() function can be called multiple times by admin after a quest ends, resulting in more than the protocolFee being paid. This will help to prevent potential abuse or accidental calling the function more than once.
Suggested Fix: include a boolean check e.g. feeWithdrawn = True so that the withdrawFee() function can only be called once
/// In Quest.sol /// @notice add bool bool public feeWithdrawn;
/// @notice Starts the Quest /// @dev Only the owner of the Quest can call this function function start() public virtual onlyOwner { isPaused = false; hasStarted = true; feeWithdrawn = false; }
/// In Erc20Quest.sol
/// @notice Sends the protocol fee to the protocolFeeRecipient
/// @dev Only callable when the quest is ended
function withdrawFee() public onlyAdminWithdrawAfterEnd {
if (feeWithdrawn) revert FeeAlreadyWithdrawn();
IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee());
feeWithdrawn = true;
}
#0 - c4-judge
2023-02-06T23:28:20Z
kirk-baird marked the issue as duplicate of #23
#1 - c4-judge
2023-02-14T08:54:05Z
kirk-baird marked the issue as satisfactory
🌟 Selected for report: RaymondFam
Also found by: 0xMirce, AkshaySrivastav, AlexCzm, Aymen0909, BClabs, CodingNameKiki, ElKu, HollaDieWaldfee, Josiah, KIntern_NA, MiniGlome, StErMi, adriro, bin2chen, cccz, chaduke, csanuragjain, gzeon, hihen, holme, libratus, minhquanym, omis, peakbolt, peanuts, rbserver, rvierdiiev, timongty, ubermensch, usmannk, wait, zaskoh
7.046 USDC - $7.05
Judge has assessed an item in Issue #664 as 3 risk. The relevant finding follows:
[L-2] ERC1155 Quest: withdrawRemainingTokens should factor in total number of receipts minted before withdrawal
Issue: There may be users with unredeemed receipts who will not be able to claim if all the remaining tokens are withdrawn to the owner after a quest has ended. Ideally, the process should be the same as ERC20 where this is taken into account.
Suggested Fix: Include the Quest Factory contract in the ERC1155 contract as well (e.g. through the constructor similar to receiptContractAddress_)
In withdrawRemainingTokens() function:
uint unclaimedTokens = (receiptRedeemers() - redeemedTokens); uint256 nonClaimableTokens = IERC1155(rewardToken).balanceOf(address(this)) - unclaimedTokens; IERC1155(rewardToken).safeTransferFrom(address(this), to_, nonClaimableTokens, nonClaimableTokens, '0x00'); /// @notice Call the QuestFactory contract to get the amount of receipts that have been minted /// @return The amount of receipts that have been minted for the given quest function receiptRedeemers() public view returns (uint256) { return questFactoryContract.getNumberMinted(questId); }
Link to Github Reference: https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc1155Quest.sol#L52-L63
#0 - c4-judge
2023-02-06T23:29:02Z
kirk-baird marked the issue as duplicate of #42
#1 - c4-judge
2023-02-10T05:03:11Z
This auto-generated issue was withdrawn by kirk-baird
#2 - c4-judge
2023-02-10T05:12:14Z
This previously downgraded issue has been upgraded by kirk-baird
#3 - c4-judge
2023-02-14T09:24:22Z
kirk-baird marked the issue as satisfactory
#4 - c4-judge
2023-02-23T23:49:21Z
kirk-baird changed the severity to 2 (Med Risk)