StRSR era changes can be leveraged for governance attacks

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSRVotes.sol#L95 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/plugins/governance/Governance.sol#L185

Vulnerability details

The Governance contract bases the weight it gives to users' votes on the balance they held at the time the proposal was opened up for voting:

File: Governor.sol
565:     function _castVote(
---
571:     ) internal virtual returns (uint256) {
572:         ProposalCore storage proposal = _proposals[proposalId];
573:         require(state(proposalId) == ProposalState.Active, "Governor: vote not currently active");
574: 
575:         uint256 weight = _getVotes(account, proposal.voteStart, params);
576:         _countVote(proposalId, account, support, weight, params);
---
File: Governance.sol
166:     function _getVotes(
167:         address account,
168:         uint256 timepoint,
169:         bytes memory /*params*/
170:     ) internal view override(Governor, GovernorVotes) returns (uint256) {
171:         return token.getPastVotes(account, timepoint); // {qStRSR}
172:     }

The getPastVotes function gets the StRSR balance of a user at the given timepoint.

Because of how StRSR staking eras are implemented, a RSR seizing action can reset everyone's balance, and their voting power with them.

Proof of Concept

This opens up for a possible attack vector, starting from a specific scenario:

• an RToken is undercollateralized,
• its stakeRate is close to its MAX_STAKE_RATE value
• the next action for the RToken to re-collateralize is to seize RSR

In this scenario, a malicious actor can submit a malicious proposal (or multiple ones over time) that will start in MIN_VOTING_DELAY (1 day) or after.

After this time is almost over:

• the actor can call BackingManager.rebalance (as it's permissionless)
• the rebalance causes the era to be reset, so no-one holds StRSR balance
• immediately after, the actor can deposit RSR
• optionally, the actor can stuff all blocks leading to the voting start for the proposal, preventing others from gaining any voting power
• they can cast their vote, which can't be countered because at the time voting started, no voting power was available to anyone else than the attacker

File: Governor.sol
206:     /**
207:      * @dev See {IGovernor-proposalSnapshot}.
208:      */
209:     function proposalSnapshot(uint256 proposalId) public view virtual override returns (uint256) {
210:         return _proposals[proposalId].voteStart;
211:     }

Impact

RTokens are vulnerable to rare and elaborate, but still possible, governance attacks.

Tools Used

Code review, Foundry

Recommended Mitigation Steps

Consider modifying the Governance.startedInSameEra function to also require that the proposal was created in the current era.

This will give honest participants at least one full day after an era reset to build up a fair and representative voting power.

Assessed type

Governance

[tbrent]

dupe with #17

[tbrent]

Known issue -- see page 47: https://github.com/reserve-protocol/protocol/blob/72fc1f6e41da01e733c0a7e96cdb8ebb45bf1065/audits/Trust%20Security%20-%20Reserve%20Audit%20Report%203_1_0.pdf

[c4-judge]

thereksfour marked the issue as unsatisfactory: Out of scope

[3docSec]

Hi @thereksfour and @tbrent,

I agree this and #17 should be duped together; I however would like to point out that the issues pointed by the Trust audit are different:

The function documentation describes a standoff scenario whereby griefers can stake enough RSR to vote against the reset.

I guess there is no question about this one being a different topic;

In addition, if Governance parameters are chosen unsafely, there may be insufficient time for users to stake again after an era change such that an attacker could more easily attack the Governance by staking a large amount of RSR.

This second issue is more similar, but highlights a different root cause:

• era change resets everyone's stake
• a single actor can stake RSR leaving little time for others to build up sufficient countering power

This report and #17 are instead, in my opinion, a different issue, because:

• the Trust report does not mention the key aspect of a malicious proposal that is coincident with the stake reset
• the attack presented here and #17 can work regardless of Governance settings and with arbitrarily small RSR staked, if executed with the block stuffing mentioned in the PoC, or at the end of the last block before voting starts
• the fact that it's a bug and not a systemic issue is in my opinion also demonstrated by the fact that there is a proper fix, which both this report and #17 highlighted
• only once the proper fix is applied, we can say that the governance is safe when it has sensible parameters (including a sufficiently high votingDelay)

[c4-judge]

thereksfour marked the issue as duplicate of #17