[Audit Fix] Penalties can be avoided by a bad-behaving DAO member

[dmvt]

Penalize is unnecessarily strict.

Risk Rating

1 (low)

Vulnerability Details

When calling the penalize function, you have to use an amount that is less than or equal the available lambda, otherwise the SafeMath.sub in updateBalance will fail and the transaction will revert.

ElasticDAO.sol:

function penalize(address[] memory _addresses, uint256[] memory _amounts)
    for (uint256 i = 0; i < _addresses.length; i += 1) {
      tokenContract.burnShares(_addresses[i], _amounts[i]);
    }
  }
ElasticGovernanceToken.sol
  function burnShares(address _account, uint256 _amount)
..
    _burnShares(_account, _amount);
 
function _burnShares(address _account, uint256 _deltaLambda) internal {
…
    tokenHolder = _updateBalance(tokenHolder, false, _deltaLambda);

 function _updateBalance(
..
      _tokenHolder.lambda = SafeMath.sub(_tokenHolder.lambda, _deltaLambda);

Impact

If someone, who is about to be penalized, is able to front run this transaction with a call to the exit function, with only a tiny amount, then the penalize will fail.

Tools Used

Remix

Recommended Mitigation Steps

If the lambda left for the tokenholder is less than the amount to be penalized for, penalize for the entire available lambda.

Definition of Done

• ElasticDAO.penalize fires an event for any account that cannot be penalized
• failure to process the penalty for 1 bad actor does not prevent the rest from being processed
• accounts are penalized for the entire balance or the amount passed, whichever is less
• tests exist for the event and to prove the above