search

code-423n4 / 2021-11-nested-findings

1 stars 1 forks source link

FeeSplitter: No sanity check to prevent shareholder from being added twice. #135

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

GreyArt

Vulnerability details

Impact

It is possible for duplicate shareholders to be added. These shareholders will get more than intended when _sendFee() is called.

Recommended Mitigation Steps

Ensure that the _accounts array is sorted in setShareholders().

for (uint256 i = 0; i < _accounts.length; i++) {
    if (i > 0) {
        require(_accounts[i - 1] < _accounts[i], "FeeSplitter: ACCOUNTS_NOT_SORTED");
    }
    _addShareholder(_accounts[i], _weights[i]);
}
adrien-supizet commented 2 years ago

Duplicate #231

adrien-supizet commented 2 years ago

Indeed there is a fix to do here, we'll prevent adding the same shareholders instead as suggested in #231

alcueca commented 2 years ago

Taking this issue as the principal, and raising #231 to medium severity.