The StRSRP1 contract contains a critical vulnerability in its seizeRSR() function that allows the BackingManager to seize more RSR than intended. This function is designed to allow the BackingManager to seize a specified amount of RSR from the contract, adjusting the stakeRSR and draftRSR balances accordingly. However, the current implementation uses the total RSR balance of the contract (rsrBalance) in its calculations, which can be manipulated to skew the results.
The core issue lies in the calculation of stakeRSRToTake and draftRSRToTake. These values are computed using the following formulas:
The problem arises because rsrBalance includes the amount of RSR that is being seized (rsrAmount). This means that if an attacker were to transfer additional RSR to the contract immediately before calling seizeRSR(), they could artificially inflate rsrBalance, leading to an incorrect calculation of the amounts to be seized.
For example, if the contract initially has 1000 RSR, and the BackingManager attempts to seize 100 RSR, the calculation would be based on a total balance of 1000 RSR. However, if an attacker transfers an additional 9000 RSR to the contract just before the seizure, the calculation would now be based on a total balance of 10,000 RSR, significantly altering the proportion of RSR seized from each pool.
Impact
It allows the BackingManager to seize more RSR than intended, which can lead to significant financial loss and instability in the system. An attacker could manipulate the rsrBalance by transferring RSR to the contract just before calling seizeRSR(), leading to incorrect calculations and potentially allowing the BackingManager to seize more RSR than intended.
Proof of Concept
Initially, the contract has 1000 RSR total, with 500 in stakeRSR and 500 in draftRSR.
The BackingManager prepares to seize 100 RSR by calling seizeRSR(100).
Just before the transaction is processed, an attacker transfers 9000 RSR to the contract.
The seizeRSR() function calculates stakeRSRToTake as:
(500 * 100 + 9999) / 10000 = 55 (rounded up)
Similarly, draftRSRToTake is calculated as 55.
The total seized amount is 110 RSR, which is more than the intended 100 RSR.
The remaining balances are skewed: stakeRSR = 445, draftRSR = 445, instead of the expected 450 each.
Tools Used
Manual review
Recommended Mitigation Steps
To mitigate this vulnerability, the calculation of stakeRSRToTake and draftRSRToTake should be based on the proportion of rsrAmount to the total RSR in the contract before the seizure, rather than including rsrAmount in the denominator. Here's the recommended fix:
This change ensures that the calculation is based on the pre-seizure balance, preventing manipulation through last-minute RSR transfers to the contract.
Lines of code
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSR.sol#L424-L479
Vulnerability details
Description
The
StRSRP1
contract contains a critical vulnerability in itsseizeRSR()
function that allows theBackingManager
to seize more RSR than intended. This function is designed to allow theBackingManager
to seize a specified amount of RSR from the contract, adjusting thestakeRSR
anddraftRSR
balances accordingly. However, the current implementation uses the total RSR balance of the contract (rsrBalance
) in its calculations, which can be manipulated to skew the results.The core issue lies in the calculation of
stakeRSRToTake
anddraftRSRToTake
. These values are computed using the following formulas:The problem arises because
rsrBalance
includes the amount of RSR that is being seized (rsrAmount
). This means that if an attacker were to transfer additional RSR to the contract immediately before callingseizeRSR()
, they could artificially inflatersrBalance
, leading to an incorrect calculation of the amounts to be seized.For example, if the contract initially has 1000 RSR, and the
BackingManager
attempts to seize 100 RSR, the calculation would be based on a total balance of 1000 RSR. However, if an attacker transfers an additional 9000 RSR to the contract just before the seizure, the calculation would now be based on a total balance of 10,000 RSR, significantly altering the proportion of RSR seized from each pool.Impact
It allows the
BackingManager
to seize more RSR than intended, which can lead to significant financial loss and instability in the system. An attacker could manipulate thersrBalance
by transferring RSR to the contract just before callingseizeRSR()
, leading to incorrect calculations and potentially allowing theBackingManager
to seize more RSR than intended.Proof of Concept
stakeRSR
and 500 indraftRSR
.BackingManager
prepares to seize 100 RSR by callingseizeRSR(100)
.seizeRSR()
function calculatesstakeRSRToTake
as:(500 * 100 + 9999) / 10000 = 55
(rounded up)draftRSRToTake
is calculated as 55.stakeRSR
= 445,draftRSR
= 445, instead of the expected 450 each.Tools Used
Manual review
Recommended Mitigation Steps
To mitigate this vulnerability, the calculation of
stakeRSRToTake
anddraftRSRToTake
should be based on the proportion ofrsrAmount
to the total RSR in the contract before the seizure, rather than includingrsrAmount
in the denominator. Here's the recommended fix:This change ensures that the calculation is based on the pre-seizure balance, preventing manipulation through last-minute RSR transfers to the contract.
Assessed type
Other