[Audit Fix] Token.updateNumberOfTokenHolders can be invoked by anyone

[dmvt]

Summary

updateNumberOfTokenHolders does not have any authorizations on the caller so anyone can set any arbitrary value of numberOfTokenHolders for any token they specify.

Risk Rating

2

Vulnerability Details

function updateNumberOfTokenHolders in the contract Token is invoked by function _updateNumberOfTokenHolders in contract ElasticGovernanceToken. However, updateNumberOfTokenHolders does not check who is the caller so it is possible for anyone to invoke it directly and set any value.

Impact

This function is only used to update and later view the number of token holders so the funds are not at risk. In theory, there are some hypothetical scenarios, for example, someone sets numberOfTokenHolders to MAX_UINT, then the next time _updateNumberOfTokenHolders will try to increment this number, it will overflow and revert thus making the tx fail. Of course, then you need to manually set the correct value and the loop continues.

Proof of Concept

https://github.com/code-423n4/code-contests/blob/dcde6b1d78d84d0165d2defd6e959d59ff8aba68/contests/02-elasticdao/contracts/models/Token.sol#L94-L99

Tools Used

Just a simple code review using a text editor.

Recommended Mitigation Steps

Add requirements for the caller similar to the ones that are present in the function serialize.

Definition of Done

• restrict the function to only be called by the ElasticGovernanceToken