code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

TurboRouter: Dangerous PeripheryPayments Contract #25

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/fei-protocol/ERC4626/blob/5b786fe0317f65f5b716f577c28092fa349c4903/src/external/PeripheryPayments.sol

Vulnerability details

Impact

As an entry contract, the TurboRouter contract plays an important role in interacting with users. And the TurboRouter contract inherits from the PeripheryPayments contract. In the PeripheryPayments contract, anyone can use the tokens and ETH in the contract through the approve, wrapWETH9, unwrapWETH9, sweepToken, refundETH functions. This also means that after a user mistakenly transfers tokens or ETH to the TurboRouter contract, anyone can withdraw those tokens or ETH.

Proof of Concept

https://github.com/fei-protocol/ERC4626/blob/5b786fe0317f65f5b716f577c28092fa349c4903/src/external/PeripheryPayments.sol

Tools Used

None

Recommended Mitigation Steps

Add access control to the approve, wrapWETH9, unwrapWETH9, sweepToken, refundETH functions in the PeripheryPayments contract or delete unnecessary functions directly.

Joeysantoro commented 2 years ago

These functions are necessary. The router is not meant to hold any user tokens, so mistaken transfers would be taken immediately by MEV. This is not an issue

GalloDaSballo commented 2 years ago

Agree with the sponsor, the router is a permissionless utility contract, it's not meant to hold any funds and any fund left in it will rightfully get called out