code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

The Burn function of DpxEthToken.sol will be inaccessible by any address #260

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/dpxETH/DpxEthToken.sol#L19-L26

Vulnerability details

Impact

This issue pertains to the burn and burnFrom methods. These methods have been restricted using the onlyRole(BURNER_ROLE) modifier. However, a crucial oversight exists where the BURNER_ROLE has not been initialized either in the contract's constructor or any other part of the contract. As a result, these methods are effectively inaccessible due to the missing role assignment.

Proof of Concept

The two functions are restricted to BURNER_ROLE, but the role was never set.

 function burn(
    uint256 _amount
  ) public override(ERC20Burnable, IDpxEthToken) onlyRole(BURNER_ROLE) {
    _burn(_msgSender(), _amount);
  }

  function burnFrom(
    address account,
    uint256 amount
  ) public override onlyRole(BURNER_ROLE) {
    _spendAllowance(account, _msgSender(), amount);
    _burn(account, amount);
  }

Tools Used

Manual review.

Recommended Mitigation Steps

set the BURNER_ROLE in the constructor just like the other roles.

 constructor() ERC20("Dopex Synthetic ETH", "dpxETH") {
    _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
    _grantRole(PAUSER_ROLE, msg.sender);
    _grantRole(MINTER_ROLE, msg.sender);
    _grantRole(BURNER_ROLE, msg.sender);
  }

Assessed type

Access Control

bytes032 commented 1 year ago

These can be assigned separately

c4-pre-sort commented 1 year ago

bytes032 marked the issue as low quality report

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #1840

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)