Loss of msg.value sended with TxBuilderExtension::execute in certain cases
Summary
User can lose ether if in TxBuilderExtension::execute DEFER_LIQUIDITY_CHECK action goes before actions with native tokens.
Vulnerability Detail
*_NATIVE_TOKEN actions using msg.value, but if user wants to use DEFER_LIQUIDITY_CHECK function, after the liquidity check is deferred and callbacks msg.value will be 0. For example ironBank.supply won't revert on zero supply, transaction will be executed and user supplis 0 instead of initial msg.value.
evilakela
medium
Loss of msg.value sended with TxBuilderExtension::execute in certain cases
Summary
User can lose ether if in
TxBuilderExtension::executeDEFER_LIQUIDITY_CHECK action goes before actions with native tokens.Vulnerability Detail
*_NATIVE_TOKEN actions using
msg.value, but if user wants to use DEFER_LIQUIDITY_CHECK function, after the liquidity check is deferred and callbacks msg.value will be 0. For exampleironBank.supplywon't revert on zero supply, transaction will be executed and user supplis 0 instead of initial msg.value.Impact
Loss of funds for user
Code Snippet
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L252-L256 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L100-L102 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/extensions/TxBuilderExtension.sol#L144-L148
Tool used
Manual Review
Recommendation
Simplest solutions would be just remove this functionality from batch execution and wrap ether separetly
Duplicate of https://github.com/sherlock-audit/2023-05-ironbank-judging/issues/361