rToken can avoid paying any fees by setting DAOFeeRegistry to a different contract

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Main.sol#L79-L84

Vulnerability details

Impact

rToken can avoid paying any DAO fees at all

Proof of Concept

Main.sol#L79-L84

function setDAOFeeRegistry(DAOFeeRegistry feeRegistry_) external onlyRole(OWNER) {
    require(address(feeRegistry_) != address(0), "invalid registry address");
    require(address(daoFeeRegistry) == address(0), "already set");

    daoFeeRegistry = DAOFeeRegistry(feeRegistry_);
}

setDAOFeeRegistry allows the daoFeeRegistry to be set to any address. Although this function can only ever be called once, it is neither called during creation nor upgrade. This means the creator of the rToken is allowed to set this address to whatever address they choose. The provided DAOFeeRegistry contract allows the main stRSR protocol to set fees on rTokens. By setting it to a different address the rToken is able to fully benefit from the Reserve protocol while making it impossible for any fees to be levied.

Tools Used

Manual review

Recommended Mitigation Steps

The registry addresses should be set as immutable variables on the implementation. This way, all proxies that point to the implementation will automatically inherit the proper addresses and don't have to set them.

Assessed type

Invalid Validation

[thereksfour]

After rethinking, have downgraded https://github.com/code-423n4/2024-07-reserve-findings/issues/51 to QA and will move this finding to the finding repo as QA later.

[thereksfour]

After rethinking, consider it invalid. setDAOFeeRegistry is a prerequisite for privilege escalation. That is, in this finding, the customized DAOFeeRegistry occurs before privilege escalation, which is a malicious configuration or unintentional enabling the DAOFee.