[Security] Authorization bypass via `tx-sender` spoofing

[evonide]

The dlmm-core-v-1-1.clar contract uses tx-sender for authorization, which allows malicious contracts to impersonate legitimate users when called in a transaction chain. As long as tx-sender remains unchanged, which is the case for normal contract calls without as-contract, an attacker-controlled contract can execute privileged functions on behalf of the original sender throughout the entire transaction chain. This introduces well-known spoofing risks in Clarity for example as discussed in stacks-sbtc/sbtc#500.

Please note that post-conditions cannot protect users from state-changing transactions that don't involve token transfers, as post-conditions only guard against asset movements, not arbitrary state modifications.

Attacks

The core contract differentiates between the following roles

• Deployer - CONTRACT_DEPLOYER
• Pool admins - admins-list
• Variable fees manager - variable-fees-manager

Attacks are possible against each role and authorized functions which do not transfer assets themselves. Consider this example:

bitflow-dlmm/clarity/contracts/dlmm-core-v-1-1.clar

Lines 1677 to 1694 in e2cf868

	;; Add an admin to the admins list
	(define-public (add-admin (admin principal))
	(let (
	(admins-list (var-get admins))
	(caller tx-sender)
	)
	;; Assert caller is an existing admin and new admin is not in admins-list
	(asserts! (is-some (index-of admins-list caller)) ERR_NOT_AUTHORIZED)
	(asserts! (is-none (index-of admins-list admin)) ERR_ALREADY_ADMIN)
	;; Add admin to list with max length of 5
	(var-set admins (unwrap! (as-max-len? (append admins-list admin) u5) ERR_ADMIN_LIMIT_REACHED))
	;; Print add admin data and return true
	(print {action: "add-admin", caller: caller, data: {admin: admin}})
	(ok true)
	)
	)

This first defines caller tx-sender, then performs an authorization check against it and finally changes state via:

(var-set admins (unwrap! (as-max-len? (append admins-list admin) u5) ERR_ADMIN_LIMIT_REACHED)) 

This can be exploited in any scenario where a legitimate admin calls into an attacker-controlled contract, either directly or indirectly, as long as tx-sender remains intact throughout the call chain. Multiple examples come to mind:

• Direct call into an attacker controlled contract.
• Indirect call into an attacker controlled contract through an obscure call chain. For example, a legitimate protocol which then calls into an attacker controlled contract.
• Calling into a malicious pool. The protocol intends to verify the legitimacy of pools through the contract-hash? functionality which is currently disabled (tracked per [Security] Pool verification is disabled #7) and yet to be activated.
• Executing a swap on a pool which uses a malicious token. Note: The protocol has verification for pools but not for individual tokens (tracked per [Security] Spoofing risk through potentially malicious token contracts #8). An attacker can create a legitimate pool with a malicious token. An example for where the core contract can call into such a token is swap-x-for-y here:
  

  bitflow-dlmm/clarity/contracts/dlmm-core-v-1-1.clar

  Lines 1047 to 1049 in e2cf868

  	;; Transfer dx + x-amount-fees-total x tokens from caller to pool-contract
  	(try! (contract-call? x-token-trait transfer (+ dx x-amount-fees-total) caller pool-contract none))

As per above, there are many realistic paths which can lead to impersonation and hijacking of functionalities or the full core or pool contracts.

Recommendation

We should rely on contract-caller where possible and avoid utilizing tx-sender for authorization purposes unless it's absolutely necessary.

[evonide]

This risk also applies to https://github.com/BitflowFinance/bitflow-dlmm/blob/48b4f159835231d96e23b46f03998584dcfef124/clarity/contracts/dlmm-staking-sbtc-usdc-v-1-1.clar which uses tx-sender in a similar way.