Add SIP: Composable Fungible Tokens with Allowance

[jio-gl]

Abstract

This proposal extends the SIP-010 standard trait for fungible tokens on the Stacks blockchain to support composable fungible tokens with allowances. It addresses the limitations of the previous standard, which did not provide sufficient support for composability and security. The new trait includes functions for transferring tokens, approving allowances, checking allowances, and transferring tokens using allowances. The recommended implementation of approve used incremental allowances to avoid rance conditions and double transfering.

[LNow]

This approach decrease UX, doesn't fix any security issue and creates new ones.

When we're talking about tokens (both FT and NFT), the use of tx-sender is far more risky for a very specific subset of contracts ie. DEX or NFT exchange than for ordinary user. Normal user have to sign TX created with allow-mode to be affected by it. In context of tx-sender the real danger for normal users lives in operations that don't involve assets transfers. Such as approve from this proposal.

[jio-gl]

@LNow thanks for you comment. I improved the Rationale with 3 cases where the current approach for doing a 2-hop DeFi Composability (User using a Yield Aggregator service using a Yield service). The current de-facto approach of checking tx-transfer makes Phishing much more dangerous (they can drain all the popular SIP10 tokens you have). Also the current approach makes platforms with arbitrary token pools, such as a Token Swap platforms, very dangerous for the same reason, they grab the tx-sender and they can start draining a whole lot of SIP10s.

[LNow]

In order to steal any tokens from user first you have to convince him/her to sign a transaction with either no post-conditions at all (aka. TX with allow-mode) or with post-conditions that precisely describes what tokens will be transferred. In both cases user must be dumb to sign such transaction.

With your proposal - all you have to do is convince user to sign TX that will not move any tokens (allowance TX have no side effects). And it is much easier than you might think.

When it comes to draining Token Swap platforms, I've covered this topic quite a few times (with some projects). There are ways to build contracts in such way, that it is almost impossible to drain them, and even if it is possible you can limit it to single pair of assets. You just need to understand the security model used in Stacks. In my opinion this proposal tries to bypass this model without bringing any real benefits (from the security standpoint).

1) Allowance model forces user to give away the control of his/her tokens to someone else. 2) It decrease the UX, as it forces users to sign 2 separate transactions 3) It decrease the security, as it forces users to think to whom and for how long they are giving the power to control all or portion of their holdings. 4) It decrease the flexibility and composability. If project decides to add new functionality to their product they will have to ask users to grant them new allowance, simply because they deployed a new contract with a new feature. 5) It creates HUGE security risk for users, because instead of relying on security model built in into the ecosystem (post-conditions) they will have to trust contracts that 99.99999% have no idea how to read, nor analyze.

IMHO a lot of cons and risks, and no real benefits.

[friedger]

Post conditions in clarity for contract-call? would be cool.

[friedger]

Can we evolve this SIP to a post about best practices for defi protocols on stacks?

[MarvinJanssen]

With your proposal - all you have to do is convince user to sign TX that will not move any tokens (allowance TX have no side effects). And it is much easier than you might think.

This line by @LNow gets to the core of the issue. If there SIP were activated and implemented then it will become quite a bit easier to steal tokens. You can either hide a call to approve somewhere in a different contract call or convince the user to do it directly, because Hiro Wallet et al. will report "No tokens will be transferred in this call". Once the transaction is broadcast the floodgates are opened. The tx-sender guard is mainly an issue for contract principals. They are not awarded the security of post conditions and are the ones that get attacked via that route when using dynamic dispatch. That's why usually a guard like (or (is-eq tx-sender sender) (is-eq contract-caller sender)) is enough to mitigate it; for, contracts will no longer need to resort to as-contract. To solve the issue once and for all and keep composability we would need a construct in Clarity that allows contracts to specify post conditions for contract-call? as mentioned by @friedger.

[jio-gl]

@MarvinJanssen you cannot call approve allowance from another contract because only the contract caller can approve its own allowance. If they convince you on Phishing to send a approve allowance it will always be for 1 single contract. With the current approach they can literally sell the control of your tx-sender in a Phishing market once they convince you and drain any SIP10. I agree the post conditions do no add much safety on composability by being in the UI, they should be in the contract-call?. If you are doing a service aggregator, like a Yield Aggregator, is not very convenient to add UI Post-conditions to 3rd party calls, maybe you do not know the exact side-effects.

[LNow]

With the current approach they can literally sell the control of your tx-sender in a Phishing market once they convince you and drain any SIP10.

That is only possible if you sign a transaction created in allow-mode. If TX is created correctly (with deny-mode) then ALL token transfers must be specified in post-conditions.

If you are doing a service aggregator, like a Yield Aggregator, is not very convenient to add UI Post-conditions to 3rd party calls, maybe you do not know the exact side-effects.

Either you use proper post conditions, or you create TX in allow-mode which by default should be considered as malicious because transactions created in allow-mode can transfer ANY tokens and user accepts it (hence the name allow-mode - you allow contract to do anything)

[friedger]

If the approval checks for contract-caller, then contracts need to use as-contract and are in the same position as with transfer and we haven't won anything.

[jio-gl]

If the approval checks for contract-caller, then contracts need to use as-contract and are in the same position as with transfer and we haven't won anything.

Updated: yep, you are right xD got confused. If the user calls contract 1 and contract2 calls tokenA then there is not way checking only (is-eq contract-caller sender) will work.

[jio-gl]

With the current approach they can literally sell the control of your tx-sender in a Phishing market once they convince you and drain any SIP10.

That is only possible if you sign a transaction created in allow-mode. If TX is created correctly (with deny-mode) then ALL token transfers must be specified in post-conditions.

If you are doing a service aggregator, like a Yield Aggregator, is not very convenient to add UI Post-conditions to 3rd party calls, maybe you do not know the exact side-effects.

Either you use proper post conditions, or you create TX in allow-mode which by default should be considered as malicious because transactions created in allow-mode can transfer ANY tokens and user accepts it (hence the name allow-mode - you allow contract to do anything)

thanks for comment @LNow, I will research more on allow-node and deny-mode. Found this:

In addition, the Stacks blockchain supports an "allow" or "deny" mode for evaluating post-conditions: in "allow" mode, other asset transfers not covered by the post-conditions are permitted, but in "deny" mode, no other asset transfers are permitted besides those named in the post-conditions.

I thinks PCs are a great addendum to security but in case a third-party want to integrate my Web3 service I cannot enforce them to call my service with deny-all. For example, if the aggregator is using 100 pools from my service, they have to make sure to grab the token data for a specific tokens in the TX and add the deny-all flag.

[LNow]

From my personal notes:

In allow mode any token operations (transfer and/or burn) not covered by post-conditions are allowed. In other words user is giving permission to perform any token operations including those that might drain his wallet.

In deny mode all token operations (transfer and/or burn) not covered by post-conditions are prohibited. In other words if contract tries to perform any token operation not listed in post-conditions, or value/size of such operation would not match what is specified in post-condition, then such TX will fail.

I thinks PCs are a great addendum to security but in case a third-party want to integrate my Web3 service I cannot enforce them to call my service with deny-all.

Correct. You have no control over how TX that interact with your contract will be created. Transactions created in allow-mode are extremely risky for users and they should be (and AFAIK are) displayed by wallets, during signing process, as risky/dangerous/potentially malicious. Thus it doesn't matter how difficult it is to create proper post-conditions - you should never promote signing TX without them.

TX created in allow-mode should be reserved for a very special occasions, and users who sign them should be aware what are the risks and know what and why they are doing. 99.999% of users should not use them at all.

[jcnelson]

I thinks PCs are a great addendum to security but in case a third-party want to integrate my Web3 service I cannot enforce them to call my service with deny-all. For example, if the aggregator is using 100 pools from my service, they have to make sure to grab the token data for a specific tokens in the TX and add the deny-all flag.

@jo-tm Can you help me understand why this is a problem? Because Clarity is a decidable language, it's tractable to enumerate all of the halting states of a contract-call's execution. Thus, it's computable which token-transfers could be reached from a contract-call, so your wallet could automatically generate post-conditions for them. Am I missing something?

[jio-gl]

I thinks PCs are a great addendum to security but in case a third-party want to integrate my Web3 service I cannot enforce them to call my service with deny-all. For example, if the aggregator is using 100 pools from my service, they have to make sure to grab the token data for a specific tokens in the TX and add the deny-all flag.

@jo-tm Can you help me understand why this is a problem? Because Clarity is a decidable language, it's tractable to enumerate all of the halting states of a contract-call's execution. Thus, it's computable which token-transfers could be reached from a contract-call, so your wallet could automatically generate post-conditions for them. Am I missing something?

@jcnelson I wasn't sure that checking sender to be equal to tx-sender OR contract-caller for Web3 composability was a good enough approach for token security, but I am more convinced now after discusing. It might work this way combined with post-conditions. Like @LNow mentioned, the wallet should show a Warning if the TX is allow-all otherwise is very dangerous for phishing and Web3 service aggregators. Not sure if Web3 Devs are using any tools to generate the post-conditions or they are manually doing what they can. The as-contract approach to Web3 composability seems to also work as well but it seems to break the semantics of tx-sender. I think we can discuss this SIP extension as an alternative for the community (only newly added functions allow and transfer-from) or remove it altogether.

[jcnelson]

While checking tx-sender or contract-caller can protect the user in principal, I'm not convinced that this is the best we can do. The intent of post-conditions is that the user can stipulate what the effects of a transaction must be, regardless of what the transaction actually does. This empowers non-technical users to protect themselves from the effects of buggy or malicious code -- something that, AFAIK, no other blockchain can do. By contrast, checking tx-sender and contract-caller is something that only the contract developer can do; if the developer makes a mistake, the users suffer the consequences. Using an allowance-based system to constrain who can access your tokens is in the same category of security measures -- users must trust that someone else's code will do the Right Thing.

Now, do post-conditions today go far enough? No; I think they can be extended to do a lot more. For example, they could be extended to place constraints on who received tokens and NFTs, as well as who sent them. As another example, they could be scoped to only apply to specific contracts, or in specific functions. Both of these extensions are consistent with the intent of PCs -- they empower users to protect themselves from developers.

Does any Stacks wallet try to statically analyze the contract call graph in order to generate useful post-conditions? Not that I know of. However, this could be added due to the aforementioned reason that Clarity is decidable. Unlike Solidity, you can know the effects of a transaction before you run it. I think wallets should be in the business of doing this, but that's a topic for another day.

[LNow]

Now, do post-conditions today go far enough? No; I think they can be extended to do a lot more. For example, they could be extended to place constraints on who received tokens and NFTs, as well as who sent them. As another example, they could be scoped to only apply to specific contracts, or in specific functions. Both of these extensions are consistent with the intent of PCs -- they empower users to protect themselves from developers.

Something that might be worth to consider is a middle ground between allow and deny modes. A mode where user is protected like it is in allow mode, but we don't have to specify all post conditions for tokens held by contracts.

So if I want to swap token A for Z and to do that contract I'm interacting with has to swap A -> B -> C.....X -> Y -> Z I won't have to specify conditions for all transfers, only for token A and Z because as a user I'm interested only with these two. Of course if we add conditions for let's say C and G they also must be respected in that mode.

[jio-gl]

@jcnelson @LNow I think this is a productive discussion. Something that can also be secure in my opinion is to have post-conditions in Traits, so you implement SIP10 or a trait with something like stake(token, amount) that only grabs the specified token thanks to PCs. Then you don't rely on the Token Developer but if you aggregate Token or Web3 Services you call (impl-trait some-trait.some-trait.dangerous-method)to verify that the implementation is secure before using it.