Alchemix Transmuter

Summary

The setRouter function, which is in both StrategyArb and StrategyOp contracts, fails to revoke the allowance of the old router when a new router is set. This leads to unintended permission where the previous router retains the ability to unauthorizedly spend the protocol's funds.

Vulnerability Details

In the setRouter function, which is present in both StrategyArb and StrategyOp contracts, the code sets an allowance of type(uint256).max for the new router for the underlying token, but does not revoke the allowance of the old router. This means the old router, which also retains an allowance of type(uint256).max, continues to have spending permissions even after it is no longer being used, which is not in the protocol's intentions since the router is changed. The old router, which could also be compromised or malicious but doesn’t have to be, could still unauthorizedly spend the contract’s funds.

• NOTE: A similar finding was reported by Cyfrin in their Beefy Finance audit , the link: https://solodit.cyfrin.io/issues/strategypassivemanageruniswap-gives-erc20-token-allowances-to-unirouter-but-doesnt-remove-allowances-when-unirouter-is-updated-cyfrin-none-cyfrin-beefy-finance-markdown

StrategyArb code: https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L42
StrategyOp code: https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyOp.sol#L48

PoC

To demonstrate the existence of this vulnerability, we use two mock contracts: MockToken and MockTransmuter. These contracts simulate the actual system components for testing purposes. The MockToken contract is a very basic ERC20 implementation, used to represent both the synthetic asset and the underlying token. The MockTransmuter contract, on the other hand, is initialized with two token addresses, representing the synthetic asset and the underlying token, and serves as a mock transmuter in this setup.
MockTrasmuter:

MockToken:

In the setup, the StrategyOp contract is deployed with the synthetic asset and mock transmuter as parameters. The deploying user is granted the Management role, allowing them to invoke the setRouter function and set a new router. Initially, the router is set to 0xa062aE8A9c5e11aaA026fc2670B0D65cCc8B2858, with this address getting an allowance of type(uint256).max for the underlying token.

The test_vulnTest function demonstrates the vulnerability. It stores the initial router address (oldRouter), updates the router to a new address (newRouter) using setRouter, and verifies the update. It then checks allowances, confirming that the old router retains its allowance, even after being replaced. This unintended behavior allows the old router to retain spending permissions.

After running the test with forge test, we receive the following output, confirming that the vulnerability exists.

Impact

The previous router retains token approval even after being replaced, allowing it to unauthorizedly spend (or even steal, if it becomes malicious) the contract's funds, which is not in the protocol’s intentions since it has been updated.

Tools Used

• Manual code review

• Foundry

Recommendations

Consider updating the setRouter function to revoke the allowance of the old router before setting the new router and granting it a new allowance. By revoking the allowance of the old router before approving the new one, the protocol ensures that no unused or unintended allowances remain active.