The BackingManager takes care of the backing of its RToken by constantly balancing the amounts of the respective underlying assets with trades. However, there can only be one trade open at a time, which could lead to the DoS of the contract, if the contract were to be blocklisted by the token being out on a trade.
Proof of concept
Let's assume the rebalance function was called, and token A with a blocklist functionality would be sent to the newly created trade contract. After that, the BackingManager would be added to the blocklist of token A, therefore all transfers of this token to and from the BackingManager would revert. This would typically not be a huge problem, since if such a token were to be used, smoething like that should be expected when it comes to loss of funds.
However, the rebalance and revenueForwarding can only be done if no trades are open. Note that removing the asset from asset registry would not help. Since both trade types, be it the Dutch trade or the Gnosis trade, send tokens to the BackingManager in order for the trade to be settled, this action would be blocked by the blocklist.
Impact and likelihood
The likelihood of such an issue occuring is LOW. Since there is no other way to close trades then by settling (which transfers tokens), the impact should be considered HIGH, therefore MEDIUM severity.
Recommendation
Consider adding a functionality to force close trades callable by governance.
Lines of code
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BackingManager.sol#L107
Vulnerability details
Description
The
BackingManager
takes care of the backing of itsRToken
by constantly balancing the amounts of the respective underlying assets with trades. However, there can only be one trade open at a time, which could lead to the DoS of the contract, if the contract were to be blocklisted by the token being out on a trade.Proof of concept
Let's assume the
rebalance
function was called, and token A with a blocklist functionality would be sent to the newly created trade contract. After that, theBackingManager
would be added to the blocklist of token A, therefore all transfers of this token to and from theBackingManager
would revert. This would typically not be a huge problem, since if such a token were to be used, smoething like that should be expected when it comes to loss of funds.However, the
rebalance
andrevenueForwarding
can only be done if no trades are open. Note that removing the asset from asset registry would not help. Since both trade types, be it the Dutch trade or the Gnosis trade, send tokens to theBackingManager
in order for the trade to be settled, this action would be blocked by the blocklist.Impact and likelihood
The likelihood of such an issue occuring is LOW. Since there is no other way to close trades then by settling (which transfers tokens), the impact should be considered HIGH, therefore MEDIUM severity.
Recommendation
Consider adding a functionality to force close trades callable by governance.
Assessed type
DoS