Redundant native token--handling logic in _executeDstHook function
Description
The internal _executeDstHook function processes hook execution during the fill function call. The trusted solver provides the necessary data for the fund transferring before the hook call. This data includes the preferredToken address and the preferedDstInputAmount (amount of tokens to be transferred from the solver to the specified hook address).
The _executeDstHook function supports both native and ERC-20 tokens. As a result, it utilizes the provided msg.value as input tokens for hook execution. However, it handles the case of nonzero msg.value and preferredToken.isNativeToken() separately using if/if-else blocks. Since the msg.value > 0 condition is checked first, the block for preferredToken.isNativeToken() cannot be executed at all, making it effectively unreachable.
function _executeDstHook(
Order calldata order,
IAori.DstHook calldata hook
) internal allowedHookAddress(hook.hookAddress) returns (uint256 balChg) {
if (msg.value > 0) {
// Native token input
(bool success, ) = payable(hook.hookAddress).call{value: msg.value}("");
require(success, "Native transfer to hook failed");
} else if (hook.preferedDstInputAmount > 0) {
// ERC20 or native token input
if (hook.preferredToken.isNativeToken()) {
require(msg.value == hook.preferedDstInputAmount, "Incorrect native amount for preferred token");
(bool success, ) = payable(hook.hookAddress).call{value: hook.preferedDstInputAmount}("");
require(success, "Native transfer to hook failed");
} else {
IERC20(hook.preferredToken).safeTransferFrom(
msg.sender,
hook.hookAddress,
hook.preferedDstInputAmount
);
}
}
[...]
}Impact
The unreachable code creates unnecessary complexity.
Recommendations
We recommend simplifying the logic by relying only on the hook.preferredToken.isNativeToken() check, which implicitly covers the case where native tokens are sent. An updated implementation could look like this.
function _executeDstHook(
Order calldata order,
IAori.DstHook calldata hook
) internal allowedHookAddress(hook.hookAddress) returns (uint256 balChg) {
- if (msg.value > 0) {
- // Native token input
- (bool success, ) = payable(hook.hookAddress).call{value: msg.value}("");
- require(success, "Native transfer to hook failed");
- } else if (hook.preferedDstInputAmount > 0) {
+ if (hook.preferedDstInputAmount > 0) {
// ERC20 or native token input
if (hook.preferredToken.isNativeToken()) {
require(msg.value == hook.preferedDstInputAmount, "Incorrect native amount for preferred token");
(bool success, ) = payable(hook.hookAddress).call{value: hook.preferedDstInputAmount}("");
require(success, "Native transfer to hook failed");
} else {
+ require(msg.value == 0, "Native tokens should not be provided");
IERC20(hook.preferredToken).safeTransferFrom(
msg.sender,
hook.hookAddress,
hook.preferedDstInputAmount
);
}
- }
+ } else {
+ require(msg.value == 0, "Native tokens should not be provided");
+ }
[...]
}Remediation
This issue has been acknowledged by Aori, and fixes were implemented in the following commits: