Gas-consuming way to add shareholders

[code423n4]

Handle

palina

Vulnerability details

Impact

New shareholders can only be added by rewriting the whole shareholders[] array in the FeeSplitter::setShareholders() function, which iterates over an array rendering significant gas consumption. The function FeeSplitter::_addShareholder() adds an individual shareholder, but has private visibility and, therefore, cannot be called by the owner separately.

Proof of Concept

_addShareholder(): https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/FeeSplitter.sol#L262
setShareholders(): https://github.com/code-423n4/2021-11-nested/blob/5d113967cdf7c9ee29802e1ecb176c656386fe9b/contracts/FeeSplitter.sol#L104

Tools Used

Manual Analysis

Recommended Mitigation Steps

If the shareholders are expected to be added regularly, consider making _addShareholder() accessible to both the contract and the owner:
function addShareholder(address _account, uint256 _weight) public onlyOwner {.

[adrien-supizet]

This implementation costs a bit more gas indeed, but we decided to go with this one because it is simple and idempotent, and will not be called often.
Acknowledged.

[maximebrugel]

Duplicated : #135

[alcueca]

Not a duplicate of #135