FEI and TRIBE Redemption contest - rvierdiiev's results

Any redeemedTokens sent to this contract will be locked and effectively burned. They are not to be used in the accounting of redemptions or redeemBase

Hmmm, the warden has a point actually. Currently, the rewards tied to TRIBE tokens that were sent directly to the contract instead of via redeem() becomes unrecoverable (burnt) as well.

It's not a bad suggestion; it at least helps to split the rewards of directly sent TRIBE tokens with the remaining TRIBE holders, which acts as a form of reward recovery.

The recommended mitigation is good, but perhaps modified to be an immutable value / constant _redeemBase, then the calculation becomes

uint256 redeemedAmount = (amountIn * balance) / (_redeemBase - IERC20(_redeemedToken).balanceOf(address(this));

Keeping the medium severity as it's an indirect loss of rewards that can be recovered / re-distributed.

@HickupHH3 All TRIBE tokens have an equal claim to the funds that are locked in this contract, irrespective of what other TRIBE tokens do.

The situation described is just a special case of TRIBE tokens being locked out of circulation. It's not feasible for this contract to account for all locations where user circulating TRIBE is subsequently locked/removed from circulation and nor is it particularly desired - all TRIBE has an equal claim to the funds held on the contract. Dispute that this is an issue

Agree with the intention to keep all claims equal, and that it's not feasible to handle all outlier cases.

Implementing the suggestion would change user behaviour to claim as late as possible, since the TRIBE amount that's mistakenly sent only increases over time.

Downgrading to QA.

part of #121

First of all i would like to say that current implementation of TribeRedeemer is not going to pay redeemers equal claims. This is important because the sponsor argues that if we change the implementation like i have suggested in submission, then next users will get more rewards. Also HickupHH3 said that this incentivize people to call redeem as late as possible to get bigger reward.

I will explain why current implementation is not going to pay equal claims. This is because you can top up any exchange tokens directly(transfer to TribeRedeemer) and then next users will get bigger share than previous, because the formula uses exchange token balance to calculate reward amount. uint256 redeemedAmount = (amountIn * balance) / base; where balance is uint256 balance = IERC20(tokensReceived[i]).balanceOf(address(this));

So the assumption of sponsor about equal claims is not true. And also HickupHH3 assumption that my proposed change will push users to redeem tokens later is partly wrong. They are incentivized to do so already in current implementation(as someone can transfer some exchange token to the TribeReddemer). Why i suggested to use redeem token balance, because i saw in the code that TribeRedeemer is going to spend all exchange tokens for the redeemed tokens.

All exhange tokens(tokensReceived in the code) are sent to the TribeRedeemer contract before and there is no way for TribeRedeemer to return any funds that are still controlled by contract. That means that the purpose of TribeRedeemer is to distribute all exchange tokens to the redeemers(there is no logic to leave some part of that tokens locked in the contract, it's better to distribute everything).

Let's see 3 situations.

1. Someone top ups exchange tokens. Then TribeRedeemer will send more reward to all next users. Finally, TribeRedeemer will send users all the exchange tokens he had(so in the end when the last redeemed token is sent to TribeRedeemer, there is no exchange tokens controlled by TribeRedeemer).
2. Someone top ups redeem token(as i described in the submission). TribeRedeemer will still send users same part of exchange tokens. Finally, when all redeem tokens are controlled by TribeRedeemer, exchange token will have funds of TribeRedeemer that is not distributed among the redeemers. This funds are locked.
3. Someone top ups both exchange token and redeem token. At this time next users will have bigger reward than previous(so sponsors assumption is broken) and in the end some exchange token will still be controlled by TribeRedeemer.

I hope, that i showed that current implementation can't guarantee equal rewards for users and it will be logical to use the way that i proposed to not just burn exchange tokens but distribute them among redeemers. If sponsor wants to pay equal claims, then he need to initialize TribeRedeemer with one more param exchangeTokensPerRedeemToken array where he can provide fixed amount of tokens to pay for redeemed token and use smth like this uint256 redeemedAmount = (amountIn * exchangeTokensPerRedeemToken) / base;

only in this case it's possible to get users equal claims

In the course of my investigation to see if a user has accidentally directly sent TRIBE to the redeemer contract, I have discovered that what @rvierdiyev said about equal claims is true:

And also HickupHH3 assumption that my proposed change will push users to redeem tokens later is partly wrong. They are incentivized to do so already in current implementation(as someone can transfer some exchange token to the TribeReddemer).

The RGT reserve and timelock sent an additional ~430k DAI to the redeemer contract recently, after redemptions have begun:

• https://etherscan.io/tx/0xe0d2a879e05c8de8ebe23cef2f380fc68c1e52b21961b6650d076e9f839128f7
• https://etherscan.io/tx/0xab80a9982d45ced06f3f63e45a4fde5def2bc9270df0e639e9a9e84ba5a24528

This user who redeemed close to 17M TRIBE missed out on about 16856213429000000000000000 / 458964340000000000000000000 * 430000 ~= 15792.45 additional DAI.

Hence, it is a fact that not all claims are equal; later claims have already benefitted from additional tokens that were sent to the redeemer contract.

I've flipped flopped on my decision, but in light of what I found, my rationale for the downgrade no longer stands.

Implementing the suggestion would change user behaviour to claim as late as possible, since the TRIBE amount that's mistakenly sent only increases over time.

As @rvierdiyev pointed out, this is already true, although I would argue that implementing this suggestion would worsen the problem.

The situation described is just a special case of TRIBE tokens being locked out of circulation. It's not feasible for this contract to account for all locations where user circulating TRIBE is subsequently locked/removed from circulation and nor is it particularly desired.

I agree that the protocol should not be expected to handle all outlier cases; and that accidentally sending tokens to the redeemer contract is a special case. However, it is precisely because it is a boundary / special case that the protocol is able to handle and remedy; it most cases, it doesn't have the ability to do so. It's akin to token contracts blocking address(this) in to addresses. If a path exists to recover rewards that would've been lost together with the TRIBE tokens by redistributing it, why not?

Keeping the medium severity as it fulfils the condition: protocol leaking value (rewards lost that in this case can be remedied) with a hypothetical attack path with stated assumptions, but external requirements. This vulnerability can also be classified as a user-error bug, which have been awarded up to medium severity in previous contests Eg. overpaying with ETH.