sell reward rTokens at low price because of skiping furnace.melt

[code423n4]

Lines of code

https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/RevenueTrader.sol#L100-L104

Vulnerability details

Impact

The reward rToken sent to RevenueTrader will be sold at a low price. RSR stakers will lose some of their profits.

Proof of Concept

RevenueTraderP1.manageToken function is used to launch auctions for any erc20 tokens sent to it. For the RevenueTrader of the rsr stake, the tokenToBuy is rsr and the token to sell is reward rtoken.

There is the refresh code in the manageToken function:

} else if (assetRegistry.lastRefresh() != uint48(block.timestamp)) {
    // Refresh everything only if RToken is being traded
    assetRegistry.refresh();
    furnace.melt();
}

It refreshes only when the assetRegistry has not been refreshed in the same block.

So if the actor calls the assetRegistry.refresh() before calling manageToken function, the furnace.melt() won't been called. And the BU exchange rate of the RToken will be lower than actual value. So the sellPrice is also going to be smaller.

(uint192 sellPrice, ) = sell.price(); // {UoA/tok}

TradeInfo memory trade = TradeInfo({
    sell: sell,
    buy: buy,
    sellAmount: sell.bal(address(this)),
    buyAmount: 0,
    sellPrice: sellPrice,
    buyPrice: buyPrice
});

Tools Used

Manual review

Recommended Mitigation Steps

Refresh everything before sell rewards.

Assessed type

Context

[c4-sponsor]

tbrent marked the issue as sponsor confirmed

[c4-judge]

0xean marked the issue as satisfactory