RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Wrong addresses for buyer and seller are specified in events `ReFiBought` and `ReFiSold`

valya

Root + Impact

Description

  • The address of a contract which is an initial caller of PoolManager::unlock is specified as a buyer and seller in the ReFiSwapRebateHook::ReFiBought and ReFiSwapRebateHook::ReFiSold events. However, the intended addresses should be the addresses of users that call swap on that contract.

function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
bool isReFiBuy = _isReFiBuy(key, params.zeroForOne);
uint256 swapAmount =
params.amountSpecified < 0 ? uint256(-params.amountSpecified) : uint256(params.amountSpecified);
​
uint24 fee;
​
if (isReFiBuy) {
fee = buyFee;
@> emit ReFiBought(sender, swapAmount);
} else {
fee = sellFee;
uint256 feeAmount = (swapAmount * sellFee) / 100000;
@> emit ReFiSold(sender, swapAmount, feeAmount);
}
​
return (
BaseHook.beforeSwap.selector,
BeforeSwapDeltaLibrary.ZERO_DELTA,
fee | LPFeeLibrary.OVERRIDE_FEE_FLAG
);
}

Risk

Likelihood:

  • The issue occurs every swap because beforeSwap hook is called every swap.

Impact:

  • Wrong data is stored in logs. The events are useless since the seller's and buyer's addresses are equal and are an address of a contract.

Proof of Concept

As written in IHooks natspec, the sender is the initial msg.sender for the swap call and it is a contract since it should implement unlockCallback.

/// @notice The hook called before a swap
@> /// @param sender The initial msg.sender for the swap call
/// @param key The key for the pool
/// @param params The parameters for the swap
/// @param hookData Arbitrary data handed into the PoolManager by the swapper to be be passed on to the hook
/// @return bytes4 The function selector for the hook
/// @return BeforeSwapDelta The hook's delta in specified and unspecified currencies. Positive: the hook is owed/took currency, negative: the hook owes/sent currency
/// @return uint24 Optionally override the lp fee, only used if three conditions are met: 1. the Pool has a dynamic fee, 2. the value's 2nd highest bit is set (23rd bit, 0x400000), and 3. the value is less than or equal to the maximum fee (1 million)
function beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata hookData)
external
returns (bytes4, BeforeSwapDelta, uint24);
​

Recommended Mitigation

It should be implemented a way to retreive a user's address from a contract that calls swap. For instance, some getMsgSender() function may be implemented and this function may be called from ReFiSwapRebateHook to get the user which bought or sold tokens.

function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
bool isReFiBuy = _isReFiBuy(key, params.zeroForOne);
uint256 swapAmount =
params.amountSpecified < 0 ? uint256(-params.amountSpecified) : uint256(params.amountSpecified);
+ address msgSender = customRouter.getMsgSender();
uint24 fee;
​
if (isReFiBuy) {
fee = buyFee;
- emit ReFiBought(sender, swapAmount);
+ emit ReFiBought(msgSender, swapAmount);
} else {
fee = sellFee;
uint256 feeAmount = (swapAmount * sellFee) / 100000;
- emit ReFiSold(msgSender, swapAmount, feeAmount);
+ emit ReFiSold(msgSender, swapAmount, feeAmount);
}
​
return (
BaseHook.beforeSwap.selector,
BeforeSwapDeltaLibrary.ZERO_DELTA,
fee | LPFeeLibrary.OVERRIDE_FEE_FLAG
);
}
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Router address emitted instead of user in ReFiBought/ReFiSold events

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!