Skip to content

Cantina audit/pr 526 snapshot#545

Draft
SocksNFlops wants to merge 39 commits into
mainfrom
cantina-audit/pr-526-snapshot
Draft

Cantina audit/pr 526 snapshot#545
SocksNFlops wants to merge 39 commits into
mainfrom
cantina-audit/pr-526-snapshot

Conversation

@SocksNFlops

@SocksNFlops SocksNFlops commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Reference Snapshot for Cantina Audit - Do Not Merge

gretzke and others added 30 commits June 18, 2025 03:35
@gretzke
Initial commit: Permissioned Pools
1554604
@gretzke
Hook implementation
92fd04a
@gretzke
fix infinite loop
8249788
@gretzke
Wrapped token tests (#477)
af9ff57 
* Initial commit: wrapped token tests

* fix imports
@ericneil-sanc @gretzke
Permissioned router tests, permissioned posm tests (#479)
c6b5931 
* initial commit

* touch ups, rm shadowed vars

* bring in wrapped change

* format

* nit change to pure

* external, pure nit

* reset getCode paths

* relative imports

* fix isolated mint_clear_revert test

* use relative paths for mock allowlist

* use BaseAllowList not BaseAllowlist

* change allowList to allowlist

* move to permissioned tests folder

* all tests working

* merging new changes from permissioned-pools

* touch ups, rm shadowed vars

* bring in wrapped change

* format

* nit change to pure

* external, pure nit

* reset getCode paths

* relative imports

* fix isolated mint_clear_revert test

* use relative paths for mock allowlist

* use BaseAllowList not BaseAllowlist

* change allowList to allowlist

* move to permissioned tests folder

* all tests working

* replace mockAllowlist with permissionedPoolsBase

* rm mock allowlist

* run linter

* rm unused

* change casing on const

* reorganize vars

* add most of the tests mentioned in comments

* all paths covered

* seperate hooks contract instead of delegate router

* minor cleanup

* use all combinations of permissioned and normal in liq tests

* spacing nit

* nit, edit note

* add clarifying comment to wrapped factory balance check

* remove circular dependency from permissioned pools implementation (#482)

* initial commit

* rm fallback from permissioned v4 router, leave receive

* fix github ci

* use currency instead of ierc20 in some tests

* add comment to hooks on why verifyAllowlist is sufficient

* inherit from BaseHook contract

* remove unused import, generate gas snapshot

---------

Co-authored-by: gretzke <daniel@gretzke.de>
@ericneil-sanc
expect specific errors (#486)
8482d41
@gretzke
Permissioned Pools: Transfers by admins (#483)
8affcc9 
* Initial commit: forced transfers by admins

* fix merge conflict

* remove event
@ericneil-sanc @gretzke
add granular allow list and multiple hooks and fix revert tests (#484)
a8ae205 
* Initial commit: forced transfers by admins

* add granular allow list and multiple hooks and fix revert tests

* add coverage for receive

* permissioned v4 router test spacing nits

* change allowed hook check

* allowedHooks on pposm

* nit

* add inheritdoc to transferFrom

* fix merge conflict

* make setAllowedHook external

* fix test resulting for resolving conflict

* simplify checkAllowedHooks

* fix snapshot

---------

Co-authored-by: gretzke <daniel@gretzke.de>
@ericneil-sanc
permissioned-pools: match universalrouter interface (#488)
f596e82 
* initial commit

* linted

* make dispatch internal

* changed revert for command not implemented

* add underscore to internal/prive functions on permissioned v4 router

* applied fmt to permissioned v4 router
@gretzke
Permissioned Pools: Ability to pause swapping (#485)
3fc39d1 
* Add check whether swapping is enabled

* Gas snapshot

* Add natspec to event
@gretzke
Rename to permissions adapter (#489)
5ad7877
@gretzke
emit event when hooks are allowed/disallowed (#498)
334a58c
@gretzke
spelling fix
fbf665b
@gretzke
Fix typo
1154a7a
@gretzke
Merge branch 'main' into permissioned-pools
12bd7ce
@SocksNFlops
fix(permissioned-hooks): enforce permission-flags in both payer modes…
1210b70 
… of PermissionedV4Router + PermissionedPositionManager
@SocksNFlops
tests(permissioned-hooks): adding unit tests to validate enforcement …
13a4e4f 
…of permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager
@SocksNFlops
update gas snapshot for PermissionedV4Router
9c48968
@SocksNFlops
fixing snapshots and unused variable nit
08cdb30
@SocksNFlops
fixing snapshots one more time
6886991
@SocksNFlops
fix: validate mint recipient has LIQUIDITY_ALLOWED in PermissionedPos…
5cc4b17 
…itionManager
@SocksNFlops
fix(permissioned-position-manager): add _mapSettleAmount override for…
fabaf71 
… CONTRACT_BALANCE
@SocksNFlops
refactor(permissioned-routing): strip PermissionedV4Router to abstrac…
de71913 
…t contract
@SocksNFlops
Validating Permission-Flags in PermissionedV4Router and PermissionedP…
147e768 
…ositionManager (#523)

* fix(permissioned-hooks): enforce permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* tests(permissioned-hooks): adding unit tests to validate enforcement of permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* update gas snapshot for PermissionedV4Router

* fixing snapshots and unused variable nit

* fixing snapshots one more time

* nit: updating comments in permposm and permv4router
@SocksNFlops
Fix(PermissionedPositionManager): Adding Recipient checks to mint fun…
129db45 
…ction (#524)

* fix(permissioned-hooks): enforce permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* tests(permissioned-hooks): adding unit tests to validate enforcement of permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* update gas snapshot for PermissionedV4Router

* fixing snapshots and unused variable nit

* fixing snapshots one more time

* fix: validate mint recipient has LIQUIDITY_ALLOWED in PermissionedPositionManager
@SocksNFlops
Fix(PermissionedPositionManager): Overriding _mapSettlementAmount to …
f8b8f7d 
…use underlying permissionedToken balances (#525)

* fix(permissioned-hooks): enforce permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* tests(permissioned-hooks): adding unit tests to validate enforcement of permission-flags in both payer modes of PermissionedV4Router + PermissionedPositionManager

* update gas snapshot for PermissionedV4Router

* fixing snapshots and unused variable nit

* fixing snapshots one more time

* fix: validate mint recipient has LIQUIDITY_ALLOWED in PermissionedPositionManager

* fix(permissioned-position-manager): add _mapSettleAmount override for CONTRACT_BALANCE
@SocksNFlops
feat(permissioned-hooks): require verified adapter in beforeInitialize
246249e
@SocksNFlops
merge(permissioned-hooks): incorporate beforeInitialize fix from eco-194
12501a6
@SocksNFlops
fix(ci): format MockPermissionedRouter
ca50713
@SocksNFlops
fix(ci): format MockPermissionedRouter and fix snapshot
6b19fb5
@BhariGowda

BhariGowda commented May 29, 2026

Copy link
Copy Markdown

Security Review permissionedPools (two issues)
Reviewed against commit e40c0a3. Both issues are in the new src/hooks/permissionedPools/ subtree.

[H-01] PermissionedPositionManager._increase() not overridden — isAllowedHooks check bypassed for existing positions
_mint() is correctly overridden to enforce the hook allow-list. _increase() is not overridden, so INCREASE_LIQUIDITY falls through to base PositionManager._increase() → _modifyLiquidity() → poolManager.modifyLiquidity() without consulting isAllowedHooks.
Impact: When an admin calls setAllowedHook(currency, H, false) to revoke a compromised hook, existing position holders can still call INCREASE_LIQUIDITY depositing tokens into a pool backed by the revoked hook, with the hook's afterModifyLiquidity callback executing normally. The emergency control is defeated.
Fix: Override _increase() with the same _checkAllowedHooks + _checkRecipientAllowed guards as _mint(). Also evaluate _decrease() and _burn() a compromised hook runs on any modifyLiquidity call.

[H-02] createPermissionsAdapter is permissionless; verifyPermissionsAdapter passes on 1 wei
createPermissionsAdapter accepts arbitrary permissionedToken, initialOwner, and allowListChecker from any caller. verifyPermissionsAdapter promotes an adapter to verified status based solely on balanceOf(adapter) > 0 satisfied by a 1 wei transfer.
Once verified, _getOwner() returns the adapter's owner() as the authority for setAllowedHook, giving the attacker hook governance over any pool using their adapter as currency.
Attack (cost: 1 wei):

factory.createPermissionsAdapter(USDC, attacker, maliciousChecker)
USDC.transfer(evilAdapter, 1)
factory.verifyPermissionsAdapter(evilAdapter) passes
posManager.setAllowedHook(evilAdapter, maliciousHook, true) passes

Fix: Gate createPermissionsAdapter to the token contract's owner. Replace the balance check in verifyPermissionsAdapter with a signature from the token's owner.

Happy to provide a Foundry PoC if useful for the Cantina engagement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SocksNFlops @BhariGowda @gretzke @ericneil-sanc