The removeBeneficiary doesn't handle non existing addresses well, if a non-existing address is provided, it will remove the first added beneficiary.
The removeBeneficiary(address beneficiary) external onlyOwner function is intended to remove a beneficiary. If the beneficiary parameter of the function is not an address previously added to the beneficiaries, it will remove the first added beneficiery, because in such a case function _getBeneficiaryIndex(address _beneficiary) public view returns (uint256 _index) will return 0.
The following test demonstrates the issue:
If the owner out of mistake tries to remove an address not added to the beneficiaries it will remove the first beneficary. (Because of a vulnerability described in another finding this will make it impossible to regain funds in the contract)
Manual review, foundry test.
If address is not found _getBeneficiaryIndexshould return -1, and remove should do nothing in that case.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.