search

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

1 stars 0 forks source link

authenticate modifier can be bypassed #7

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

In TurboRouter.sol the authenticate modifier checks if the msg.sender is equal to the owner() function of an arbitrary address provided by the caller. Anyone can simply make a contract of the same type with the owner() function returning an address that equals the msg.sender bypassing the require check.

Proof of Concept

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

Tools Used

Manual code review

Recommended Mitigation Steps

The authenticate modifier should not call an owner() function of an arbitrary target contract which is giving by the caller to be used as a security check.

Joeysantoro commented 2 years ago

The router is stateless other than storing token approvals. This shouldn't be an issue as all authentication is scoped to the user.

Joeysantoro commented 2 years ago

Would like another Tribe dev to verify my logic here

transmissions11 commented 2 years ago

Yeah doesn't make sense to me either, the arbitrary contract authorizes the router to call itself, not other contracts.

GalloDaSballo commented 2 years ago

While the modifier may give that impression, it's usage: authenticate(address(safe)) is verifying that the safe is owned by the caller.

In lack of any POC, am marking the finding invalid