0x8chars - msg.value can be reused multiple times

[sherlock-admin]

0x8chars

medium

msg.value can be reused multiple times

Summary

msg.value can be reused to take ETH that is stuck on TxBuilderExtension.sol and UniswapExtension.sol

Vulnerability Detail

Since the execute() function allows you can chain multiple function calls in a single call e.g. TxBuilderExtension.repayNativeToken() or UniswapExtension.supplyNativeToken(), you can reuse msg.value multiple times by calling these functions multiple times but only providing enough eth for a single action. This is the same vulnerability that samczsun found in sushi’s miso contracts.

Assume that there is 1 ETH stuck on the TxBuilderExtension.sol contract. A user can call execute() with the following actions [supplyNativeToken, supplyNativeToken, redeemNativeToken] and a msg.value of 1 eth. The first supplyNativeToken uses the eth that is transferred by the user. The second supplyNativeToken uses the eth that is stuck on the contract. The redeemNativeToken is used to redeem 2 ETH.

Impact

Both the TxBuilderExtension.sol and UniswapExtension.sol contracts implement the seizeNative() function to retrieve ETH that is stuck on these contracts. This function is gated by an onlyOwner() modifier. However, as a result of this vulnerability, a user is also able to bypass this restriction to retrieve the stuck funds.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L253

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L291

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/UniswapExtension.sol#L461

Tool used

Manual Review

Recommendation

Use a local variable to track msg.value. When a function that uses msg.value is executed, subtract from this local variable instead of reusing msg.value.

Duplicate of #198