code-423n4 / 2022-02-nested-findings

0 stars 0 forks source link

Shareholders can be deleted #22

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L120

Vulnerability details

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L120 Make the require statement first line of the function, always check for empty structures before taking action. Otherwise you'll delete the existing shareholder but will have no shareholders as empty input can be passed.

maximebrugel commented 2 years ago

We are checking that the length is != 0:

require(accountsLength != 0 && accountsLength == _weights.length, "FS: INPUTS_LENGTH_MUST_MATCH");
harleythedogC4 commented 2 years ago

Agree with sponsor. I believe the warden is forgetting that if the function reverts then the delete operation will not persist (so the order does not matter here)