elasticdao/contracts

[Audit Fix] modifier onlyDAO also allows minter

Closed this issue · 0 comments

dmvt commented

Summary

modifier onlyDAO requires that the msg.sender is a daoAddress OR minter while the name suggests that it should only allow the daoAddress.

Risk Rating

3

Vulnerability Details

solidity
modifier onlyDAO() {
  require(msg.sender == daoAddress || msg.sender == minter, 'ElasticDAO: Not authorized');
  _;
}

Impact

This allows the minter to call functions that are supposed to be called only by DAO like setBurner or setMinter.

Proof of Concept

https://github.com/code-423n4/code-contests/blob/4db2720312f0958f2e89f6207a6774c9e5360655/contests/02-elasticdao/contracts/tokens/ElasticGovernanceToken.sol#L31-L34

Tools Used

Just a simple code review using a text editor.

Recommended Mitigation Steps

Remove this condition: || msg.sender == minter

Definition of Done

  • The additional || msg.sender == miner is removed