Summary
The RAACToken::setFeeCollector, RAACToken::setSwapTaxRate, RAACToken::setBurnTaxRate functions have restricted access to onlyOwner that is set up in the constructor. The RAACToken contract defines also a onlyMinter for resticting some functions only to the minter.
The RAACMinter contract includes functions to modify critical parameters of the RAACToken contract (RAACMinter::setFeeCollector, RAACMinter::setSwapTaxRate, RAACMinter::setBurnTaxRate). However, these functions in RAACToken are protected by the onlyOwner modifier, so these functions can't be called by the minter. So there isn't a way to update these parameters in the system.
Vulnerability Details
RAACMinter.sol
function setSwapTaxRate(uint256 _swapTaxRate) external onlyRole(UPDATER_ROLE) {
if (_swapTaxRate > 1000) revert SwapTaxRateExceedsLimit();
@> raacToken.setSwapTaxRate(_swapTaxRate);
emit ParameterUpdated("swapTaxRate", _swapTaxRate);
}
function setBurnTaxRate(uint256 _burnTaxRate) external onlyRole(UPDATER_ROLE) {
if (_burnTaxRate > 1000) revert BurnTaxRateExceedsLimit();
@> raacToken.setBurnTaxRate(_burnTaxRate);
emit ParameterUpdated("burnTaxRate", _burnTaxRate);
}
function setFeeCollector(address _feeCollector) external onlyRole(UPDATER_ROLE) {
if (_feeCollector == address(0)) revert FeeCollectorCannotBeZeroAddress();
@> raacToken.setFeeCollector(_feeCollector);
emit ParameterUpdated("feeCollector", uint256(uint160(_feeCollector)));
}
RAACToken.sol
@> function setFeeCollector(address _feeCollector) external onlyOwner {
if(feeCollector == address(0) && _feeCollector != address(0)){
emit FeeCollectionEnabled(_feeCollector);
}
if (_feeCollector == address(0)){
emit FeeCollectionDisabled();
}
feeCollector = _feeCollector;
emit FeeCollectorSet(_feeCollector);
}
@> function setSwapTaxRate(uint256 rate) external onlyOwner { _setTaxRate(rate, true); }
@>function setBurnTaxRate(uint256 rate) external onlyOwner { _setTaxRate(rate, false); }
Impact
Add foundry to the project following this procedure
Create a file named Minter.t.sol and copy/paste this
pragma solidity ^0.8.0;
import {Test, console2} from "forge-std/Test.sol";
import {RAACMinter} from "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import {RAACToken} from "../contracts/core/tokens/RAACToken.sol";
contract MinterTest is Test {
RAACMinter public minter;
RAACToken public raacToken;
address minterOwner = makeAddr("minterOwner");
address raacTokenOwner = makeAddr("raacTokenOwner");
address stabilityPool = makeAddr("stabilityPool");
address lendingPool = makeAddr("lendingPool");
address newFeeCollector = makeAddr("newFeeCollector");
uint256 SWAP_TAX_RATE = 100;
uint256 BURN_TAX_RATE = 50;
function setUp() public {
vm.warp(block.timestamp + 1 days + 1);
raacToken = new RAACToken(raacTokenOwner, SWAP_TAX_RATE, BURN_TAX_RATE);
minter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), minterOwner);
console2.log("raacToken: ", address(raacToken));
console2.log("minter: ", address(minter));
}
function test_MinterCantCallRaacTokenFunctions() public {
console2.log(minter.hasRole(minter.UPDATER_ROLE(), minterOwner));
vm.prank(raacTokenOwner);
raacToken.setMinter(address(minter));
vm.startPrank(minterOwner);
minter.setSwapTaxRate(110);
minter.setBurnTaxRate(55);
minter.setFeeCollector(newFeeCollector);
vm.stopPrank();
}
}
Run forge test --match-test test_MinterCantCallRaacTokenFunctions -vvvv
Log:
RAACMinter::setSwapTaxRate(110)
│ ├─ [512] RAACToken::setSwapTaxRate(110)
│ │ └─ ← [Revert] OwnableUnauthorizedAccount(0x2e234DAe75C793f67A35089C9d99245E1C58470b)
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x2e234DAe75C793f67A35089C9d99245E1C58470b)
└─ ← [Revert] OwnableUnauthorizedAccount(0x2e234DAe75C793f67A35089C9d99245E1C58470b)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 8.92ms (1.37ms CPU time)
The test shows that the call to RAACToken::setSwapTaxRate fails to unauthorized access. The same happens to the functions RAACToken::setBurnTaxRate and RAACToken::setFeeCollector if we comment the setSwapTaxRateand then setBurnTaxRate.
Tools Used
Manual review
Recommendations
Modify the access control of the RAACToken::setFeeCollector, RAACToken::setSwapTaxRate, RAACToken::setBurnTaxRate with the onlyMinter.
- function setFeeCollector(address _feeCollector) external onlyOwner {
+ function setFeeCollector(address _feeCollector) external onlyMinter {
// Fee collector can be set to zero address to disable fee collection
if(feeCollector == address(0) && _feeCollector != address(0)){
emit FeeCollectionEnabled(_feeCollector);
}
if (_feeCollector == address(0)){
emit FeeCollectionDisabled();
}
feeCollector = _feeCollector;
emit FeeCollectorSet(_feeCollector);
}
- function setSwapTaxRate(uint256 rate) external onlyOwner { _setTaxRate(rate, true); }
+ function setSwapTaxRate(uint256 rate) external onlyMinter { _setTaxRate(rate, true); }
- function setBurnTaxRate(uint256 rate) external onlyOwner { _setTaxRate(rate, false); }
+ function setBurnTaxRate(uint256 rate) external onlyMinter { _setTaxRate(rate, false); }
If you run now the test again: forge test --match-test test_MinterCantCallRaacTokenFunctions -vv
Logs:
raacToken: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
minter: 0x2e234DAe75C793f67A35089C9d99245E1C58470b
true
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.06ms (462.79µs CPU time)
All works fine.
