code423n4 commented 2 years ago

Handle

gpersoon

Vulnerability details

Impact

The function setAdvanceIncentive of DAO.sol doesn't check for a maximum value of incentive. If incentivewould be very large, then advanceIncentive would be very large and the function advance() would mint a large amount of malt.

The function setAdvanceIncentive() can only be called by an admin, but a mistake could be made. Also if an admin would want to do a rug pull, this would be an ideal place to do it.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/d3f6a57ba6694b47389b16d9d0a36a956c5e6a94/src/contracts/DAO.sol#L98-L104

  function setAdvanceIncentive(uint256 incentive)  externalonlyRole(ADMIN_ROLE, "Must have admin role") {
   ...
    advanceIncentive = incentive;

https://github.com/code-423n4/2021-11-malt/blob/d3f6a57ba6694b47389b16d9d0a36a956c5e6a94/src/contracts/DAO.sol#L55-L63

function advance() external {
...
    malt.mint(msg.sender, advanceIncentive * 1e18);

Tools Used

Recommended Mitigation Steps

Check for a reasonable maximum value in advance()

0xScotch commented 2 years ago

Definitely need to guard against arbitrarily large incentives. Disagree the risk is medium though.

GalloDaSballo commented 2 years ago

Agree with the finding, this is an example of admin privilege, where the admin can set a variable which can be used to dilute the token and rug the protocol.

Because this is contingent on the admin's action, I believe medium severity to be proper