KeeperHub / KeeperHub/keeperhub

Uniswap swap actions expose recipient and amountIn sentinels with no guard

Open Beginner friendly
#2,499 0 comments 0 reactions 0 assignees View on GitHub
accepted bug confirmed
Dominant language
TypeScript
Stars
24
Forks
93
Avg merge
1d 8h
Merged PRs (30d)
266

Description

Found while reviewing #2474, which adds a guard for the same class of sentinel on `collect`'s recipient. The swap actions in the same protocol have two more and are unguarded.

**Reason.** Uniswap's `V3SwapRouter` treats specific parameter values as flags rather than as data:

- `recipient == address(1)` is rewritten to `msg.sender`, and `recipient == address(2)` is rewritten to the router itself (`swap-router-contracts/contracts/V3SwapRouter.sol:84-85`, `:178-179`).
- `amountIn == 0` is the `CONTRACT_BALANCE` flag, which swaps the router's entire held balance of `tokenIn` (`:112-117`, `:133-139`).

`swap-exact-input` and `swap-exact-output` (`protocols/uniswap-v3.ts:507` onward) expose `recipient` and `amountIn` with no guard on either. A user typing `0x0000...0002` into Recipient Address leaves the swap output sitting in the router, where `PeripheryPayments.sweepToken` is public with no access control, so the first sweeper takes it. The action reports success.

This is the same shape as the zero-recipient case #2474 guards: a shape-valid value that silently does something other than what the field says. `lib/protocol-input-guards.ts`'s own docstring describes exactly that as the reason the module exists.

**Scope.** `lib/protocol-input-guards.ts` and the two swap action definitions in `protocols/uniswap-v3.ts`. No ABI change, no migration, no response-shape change.

Not in scope: `collect`'s recipient, which #2474 handles; the position-manager-as-recipient case, which I raised on #2474 as an extension of that guard; and whether the guards should move behind a shared registry, which is worth doing once there are more than a handful.

**Plan.** Add guard entries refusing `recipient` equal to `address(1)` or `address(2)` on both swap actions, and refusing `amountIn` of zero on `swap-exact-input`. The sentinel addresses are chain-independent constants, so unlike the position-manager case this needs nothing threaded through.

One thing to settle in review rather than assume: whether any legitimate workflow wants the `CONTRACT_BALANCE` behaviour. I do not think so - the router should never be holding a balance between transactions - but refusing zero closes a door, so it is worth someone saying that out loud before it ships.

Contributor guide

Open the contributing guide

Research direction

Start with the guard entries in lib/protocol-input-guards.ts and the swap-exact-input and swap-exact-output definitions in protocols/uniswap-v3.ts. Confirm the referenced V3SwapRouter sentinel behavior, then add the scoped guards and verify that recipient values address(1) and address(2), plus amountIn zero for swap-exact-input, are refused without changing the ABI or response shape.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity, typescript
Domain
blockchain, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.