sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

feelereth - updateBeneficiary function allows the owner to divert funds by changing where fees and donations are sent #85

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

updateBeneficiary function allows the owner to divert funds by changing where fees and donations are sent

Summary

The updateBeneficiary function allows the owner to change the beneficiary address, which is where any donation fees will be sent when claimed via the claimFee function.

Vulnerability Detail

By changing the beneficiary, the owner can divert donation fees to any address they want. This could allow them to improperly take funds from the protocol.

Impact

It compromises the integrity of the protocol by allowing the owner to take funds meant for donations or improperly profit from donation fees that were intended for a different beneficiary

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L26 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L92-L95 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L137 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L145-L148

Tool used

Manual Review

Recommendation

the updateBeneficiary function could be removed entirely, or it could be restricted to only allow specific trusted addresses to be set as the beneficiary.

sherlock-admin commented 1 year ago

4 comment(s) were left on this issue during the judging contest.

141345 commented:

d

0xyPhilic commented:

invalid because owner is trusted

darkart commented:

Works as designed if user pass wrong adress its user error

panprog commented:

invalid because owner is trusted