onSafeSlurp() can be called by anyone on TurboMaster.sol

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L279 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L289

Vulnerability details

Impact

In TurboMaster.sol the onSafeSlurp() function can be called directly by anyone while the logic implies that it should only be called by the slurp() function on the TurboSafe.sol contract which performs the required calculations beforehand. When onSafeSlurp() is called directly, a user can update the getTotalBoostedForVault and getTotalBoostedAgainstCollateral mappings by any amount by providing an arbitrary number for the feiAmount argument passed in. Any user should not be able to freely update state in such a way to throw off accounting values in the TurboMaster contract.

Proof of Concept

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L279

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L289

Tools Used

Manual code review

Recommended Mitigation Steps

The onSafeSlurp() function in TurboMaster.sol should only be able to be called by the TurboSafe.sol contract.

[Joeysantoro]

This is impossible, only safes created by the master can call this function successfully due to the id check on msg.sender

[GalloDaSballo]

Agree with the sponsor, only a Safe created by the master can call the function and pass the check