Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Minter and Burner Roles in RTokens Are Redundant and Unused

4rdiii

[M-01] Minter and Burner Roles in RTokens Are Redundant and Unused

Summary

The minter and burner roles in the RToken contract are defined and assigned but are never actually used in the mint and burn functions. Instead, the contract relies solely on the onlyReservePool modifier, making these roles redundant. As a result, setting them has no effect on the contract's functionality.

Vulnerability Details

The _minter and _burner variables are defined and assigned using the setMinter and setBurner functions. However, these roles are never referenced when enforcing access control for minting and burning. Instead, the contract uses the onlyReservePool modifier, which only checks if the caller is the reserve pool.

Relevant Code

Definition of Roles (Unused)

contract RToken is ERC20, ERC20Permit, IRToken, Ownable {
// Address of the Minter
address public _minter;
​
// Address of the Burner
address public _burner;

Mint and Burn Functions (Bypass Roles)

modifier onlyReservePool() {
if (msg.sender != _reservePool) revert OnlyReservePool();
_;
}
​
function mint(
address caller,
address onBehalfOf,
uint256 amountToMint,
uint256 index
) external override onlyReservePool returns (bool, uint256, uint256, uint256)
{
// Minting logic...
}
​
function burn(
address from,
address receiverOfUnderlying,
uint256 amount,
uint256 index
) external override onlyReservePool returns (uint256, uint256, uint256)
{
// Burning logic...
}

As seen above, minting and burning rely entirely on onlyReservePool, making _minter and _burner irrelevant.

Impact

  • Security Assumptions Are Broken: The existence of setMinter and setBurner functions implies that these roles should control minting and burning. However, they have no real effect.

  • Potential Confusion for Developers & Auditors: Future developers may assume these roles are functional, leading to incorrect security assumptions.

  • Unnecessary Gas Costs: Unused storage variables _minter and _burner consume unnecessary contract storage.

Proof of Concept

To confirm that _minter and _burner are unused, modify the test suite RToken.test.js::BeforeEach instructions to remove calls to setMinter and setBurner, then execute the tests.

Test Modification: Remove Role Assignments

solidity
beforeEach(async function () {
.
.
.
// Set mock lending pool as reserve pool
await rToken.setReservePool(mockLendingPool.target);
@> // await rToken.setMinter(reservePool.address);
@> // await rToken.setBurner(reservePool.address);
.
.
.
});
​
describe("Basic functionality", function() {
beforeEach(async function() {
// Set rToken in mock
await mockLendingPool.setRToken(rToken.target);
});
​
it("should allow minter (ReservePool) to mint RToken", async function () {
const mintAmount = ethers.parseEther("100");
const mintOnBehalfOf = user1.address;
const index = RAY;
​
await expect(mockLendingPool.mockMint(reservePool.address, mintOnBehalfOf, mintAmount, index))
.to.emit(rToken, "Mint")
.withArgs(reservePool.address, mintOnBehalfOf, mintAmount, index);
​
const scaledBalance = await rToken.scaledBalanceOf(mintOnBehalfOf);
expect(scaledBalance).to.equal(mintAmount);
});
​
it("should allow burner (ReservePool) to burn RToken", async function () {
const mintAmount = ethers.parseEther("100");
const index = RAY;
​
//crvUSD minted to user1
await mockLendingPool.mockMint(reservePool.address, user1.address, mintAmount, index);
​
// crvUSD minted to the rToken contract
await mockAsset.mint(rToken.target, mintAmount);
​
await expect(mockLendingPool.mockBurn(user1.address, user1.address, mintAmount, index))
.to.emit(rToken, "Burn")
.withArgs(user1.address, user1.address, mintAmount, index);
​
const scaledBalance = await rToken.scaledBalanceOf(user1.address);
expect(scaledBalance).to.equal(0);
});
});

Expected Outcome

The minting and burning functions continue to work as expected, proving that these roles have no effect.

Further Validation

Try assigning _minter and _burner to a different address and attempt to mint/burn from that address. The transaction will fail, confirming that these roles are not enforced.

Recommendation

Option 1: Remove the Unused Roles

  • If _minter and _burner are not necessary, remove them from the contract to eliminate confusion and reduce gas costs.

Option 2: Implement Proper Role Enforcement

  • Introduce new onlyMinter and onlyBurner modifiers to enforce access control.

modifier onlyMinter() {
if (msg.sender != _minter) revert OnlyMinter();
_;
}
​
modifier onlyBurner() {
if (msg.sender != _burner) revert OnlyBurner();
_;
}

  • Apply these modifiers to mint and burn functions instead of onlyReservePool.

Updates

Lead Judging Commences

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.