Platform: Code4rena
Start Date: 09/09/2022
Pot Size: $42,000 USDC
Total HM: 2
Participants: 101
Period: 3 days
Judge: hickuphh3
Total Solo HM: 2
Id: 161
League: ETH
Rank: 2/101
Findings: 2
Award: $18,933.67
🌟 Selected for report: 1
🚀 Solo Findings: 1
🌟 Selected for report: cccz
18900 USDC - $18,900.00
In the TribeRedeemer contract, the user provides redeemedToken to redeem the tokens in the list. The transaction will revert when the balance of tokens in the list is 0. This prevents the user from losing their redeemedToken if they redeem when there are no tokens in the contract.
for (uint256 i = 0; i < tokensReceived.length; i++) { uint256 balance = IERC20(tokensReceived[i]).balanceOf(address(this)); require(balance != 0, "ZERO_BALANCE"); // @dev, this assumes all of `tokensReceived` and `redeemedToken` // have the same number of decimals uint256 redeemedAmount = (amountIn * balance) / base; // 10^6 * 10^18 / 10^8*10^6 amountsOut[i] = redeemedAmount; }
However, a malicious user could send tokens in the list to the TribeRedeemer contract so that the token balance is not 0. This would allow the redeem function to work, and the user would suffer a loss when they redeem early by mistake.
None
Consider making the TribeRedeemer contract inherit the Pausable contract and allow users to redeem when a sufficient number of tokens have been sent to the contract
#0 - thomas-waite
2022-09-19T13:44:16Z
Do not understand what the issue presented is. If the attacker sends funds to the contract so the balances are not 0, then the user would be able to redeem as normal. How do they suffer any loss if they 'redeem early'?
The balances will only be 0 when all users have redeemed. Not an issue
#1 - thereksfour
2022-09-20T13:11:08Z
@thomas-waite Consider the following scenario.
for (uint256 i = 0; i < tokensReceived.length; i++) { uint256 balance = IERC20(tokensReceived[i]).balanceOf(address(this)); require(balance != 0, "ZERO_BALANCE");
#2 - HickupHH3
2022-09-28T06:55:09Z
Redemptions can only begin when the contract has non-zero balances for all redeemed tokens, but the start redemption time isn't explicitly stated.
Malicious users can break this assumption by sending paltry amounts to the contract as explained above. Naiive users might begin redemptions early, thus losing out on the tokens they would've otherwise received after the full redemption token amounts have been sent to the contract.
It is unclear if there is a time lag between the contract deployment and time at which redemption funds are sent, and if so, its duration.
While unlikely to happen, it would be a case of users losing out on rewards they should be entitled to. Hence, I'm siding with the warden in this instance.
#3 - thomas-waite
2022-10-18T13:24:42Z
@HickupHH3 , the contract gets deployed ahead of time by necessity (the address is needed for the DAO vote). The DAO vote which funds the contract executes after deployment, there is a time lag of >= 3 days.
I do not agree that the situation described is a vulnerability in the contract and instead it would be down to user/deployer error. Clearly, no user was encouraged to redeem before funds were available and the contract address was not publicised. So no user attempted to redeem before funds were available.
#4 - HickupHH3
2022-10-18T14:15:54Z
I respectfully disagree.
There is no doubt that there a number of prerequisites to enable this attack, which makes the likelihood low:
From a game theory POV, small TRIBE holders could band together to target a few large TRIBE holders.
With these external requirements, it is only then that we will see what could be deemed as "protocol leaked value".
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements
The scenario thankfully remained hypothetical.
🌟 Selected for report: GalloDaSballo
Also found by: 0x040, 0x1f8b, 0x4non, 0x52, 0x85102, 0xNazgul, 0xSky, 0xSmartContract, Aymen0909, Bnke0x0, CertoraInc, Chandr, Chom, CodingNameKiki, Deivitto, Diana, Funen, JC, Jeiwan, Junnon, KIntern_NA, Lambda, Mohandes, Noah3o6, Ocean_Sky, Picodes, R2, Randyyy, RaymondFam, ReyAdmirado, Rohan16, Rolezn, Samatak, Sm4rty, SnowMan, SooYa, StevenL, Tagir2003, Tointer, TomJ, Tomo, V_B, Waze, _Adam, __141345__, a12jmx, ajtra, ak1, asutorufos, bharg4v, bobirichman, brgltd, c3phas, cccz, cryptonue, cryptostellar5, cryptphi, csanuragjain, d3e4, datapunk, delfin454000, dipp, djxploit, durianSausage, erictee, fatherOfBlocks, gogo, got_targ, hansfriese, horsefacts, hyh, ignacio, innertia, izhuer, karanctf, ladboy233, leosathya, lucacez, lukris02, mics, oyc_109, pashov, pauliax, prasantgupta52, rbserver, ret2basic, rfa, robee, rokinot, rotcivegaf, rvierdiiev, sach1r0, scaraven, sikorico, simon135, smiling_heretic, sorrynotsorry, unforgiven, wagmi, yixxas
33.6679 USDC - $33.67
Hardcoded DAI and FEI token addresses are used in SimpleFeiDaiPSM contract, this is not a good practice, if the token address changes make the contract unusable, variables should be used as the token address
In the RariMerkleRedeemer contract, the hasNotSigned modifier should be used when the function requires the user to provide a signature, like the sign and signAndClaimAndRedeem functions, so signAndClaim should also use the hasNotSigned modifier.