All eth can be stolen from the rubiconRouter

[code423n4]

Lines of code

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/RubiconRouter.sol#L475

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

The withdrawForETH() function has a target pool address as input. This address is never validated. This means an attacker contract can be introduced as a fake pool.

The attacker contract could pass all the checks since all checks are calls to the contract.

Step by step : An attacker makes a malicious contract that implements the BathToken interface

When called targetPool.UnderlyingToken will return wethAddress to pass the check

The same can happen for targetPool.balanceOF and transferFrom

Finally targetPool.withdraw is called which doesn't withdraw anything since it's a fake pool but returns the router's ETH balance

Finally msg.sender.transfer(withdrawnWETH) happens and the attacker gets all the ETH from the contract

I consider this to be medium severity since it's not clear why the contract would hold ETH but the contract has a receive and fallback function so it's able to receive ETH.

Recommended Mitigation Steps

Add input validation for targetPool address

[KenzoAgada]

Duplicate of #356.

[bghughes]

Duplicate of #356