claimFees may end up locking user funds

[code423n4]

Handle

0xngndev

Vulnerability details

Impact

In the NestedBuybacker contract, the function claimFees is set to public, meaning anyone can call that function. The function makes a call to feeSplitter.releaseToken(), a function that calculates the amount of fees owed to the msg.sender, and transfers that amount to the msg.sender.
If user A is a shareholder who is owed fees and calls the claimFees function, he would be the msg.sender of claimFees, but the NestedBuybacker contract would be the msg.sender of the internal call claimFees does to feeSplitter.releaseToken, meaning that feeSplitter.releaseToken would transfer tokens to NestedBuybacker instead of transferring them to User A.

Furthermore, there's no dust collector or withdraw function in the Nestedbuybacker contract, meaning that if this scenario ever plays out, those funds would be locked.

Recommended Mitigation Steps

An easy solution would be to make claimFees and internal function in NestedBuybacker. If an user wants to withdraw their fees, they can always call releaseToken in the feeSplitter contract, which is also public.

[adrien-supizet]

We agree that the claimFees function should be internal and not public.
Although the behavior described is inaccurate. The FeeSplitter would always transfer the funds to the NestedBuyBacker and not to the caller of the NestedBuyBacker.

Confirmed but severity should be non critical