Enable Canisters to Hold ICP

Guess, we’ll just need to workaround lol

Could you elaborate on wasm jailbreaking? What is the risk here?

As far as “principals can update the smart contracts and ergo aren’t immutable so are vulnerable to rug pulls and other malicious smart contract creators” - this seems like a normal risk that people accept when doing smart contract operations. It’s not necessarily Excellent that this is the case - but it doesn’t seem like a risk anyone in the wider DeFi ecosystem is all too concerned about.

2 Likes

This is a great question. I can think of some other questions well:

  1. How sophisticated would this attack have to be?
  2. What level of access is required to perform this attack
  3. What resources are required
  4. Finally (most important in this case) what is a realistic timeframe for someone to actually plan for and execute this attack? Could all of that be achieved in the 3-4 month window that is afforded to the attacker before the sandboxing feature is implemented?
    4a. Is there any chance that the sandboxing feature will be delayed?
1 Like

This post is informing on one of two issues: the wasm jail-break. The subnet congestion issue is separate.

Please do go through the link for the community thought on perceived risk. IMO it would take a fairly sophisticated attack. However even the potential of an attack vector along with known method of preventing such an vector (i.e. sandboxing) , particularly in context of IC and the reputation of best cryptographic team in the world at stake, is the key issue.

The rug pull issue (alluded to by @cmatza ) is orthogonal to the sandboxing; and cannot / should not be addressed here. (It is a valid issue, though).

2 Likes

hey folks,

I deliberately let this thread breathe to see what folks thought, but overall, I think there is a consensus in doing a new NNS motion proposal for this kind of change. I particularly am attracted by the clarity of thought in @LightningLad91’s post.

I will draft an NNS motion proposal, post it on this thread, and see what folks think. Once we have a more baked version we can submit for formal NNS motions. I am sure it will have multiple areas where we will need to tweak and change, but I wanted to let folks know that I appreciate the guidance.

5 Likes

Hi @mparikh. Thanks for providing the reference. I’ve read through it before, but I read through it again to refresh myself. I also read through the latest on the Sandboxing thread here: Security Sandboxing

These threads are great for technical folk, but they don’t present the information in a digestible format for the average community member. A good security engineer should be able to synthesize the discussion into a few key points.

Furthermore, we should not be making decisions like this based on perceived risk; rather, we should put emotions aside and use quantitative and qualitative measures to provide an accurate risk assessment.

I disagree. The reputation of Dfinity should not be a driving factor (no offense intended).

You and others have already acknowledged that this vulnerability has existed for some time now. We already have millions in assets (mainly NFTs) that are susceptible to exploitation. What does this mean? It means that delaying this proposal does not mitigate the risk. Delaying this proposal simply reduces the potential impact (total $$$ stolen) of an attack. I’m not trying to be dismissive; I just want to paint an accurate picture.

Agreed. But I also think this may be understating it. We are talking about a decentralized system right? I don’t claim to be an expert on the IC architecture, but even if a rogue process was able to escape from it’s canister and tamper with another, wouldn’t this have to be accomplished on all replicas at the same time in order for any valuable assets to be moved around? I assume consensus would still have to be reached, no? This sounds like an extremely well orchestrated attack that requires a deep understanding of the IC. Does that mean it’s impossible? absolutely not. But it is something that should be measured and included in our assessment.

To be clear; i’m not saying i’m for, or against, a delay. I’m just asking for additional information and for everyone to remember that we are talking about a network whose very existence depends on an active ecosystem. What do I mean by this? Unlike other networks, users cannot keep the network alive by running their own hardware. We effectively pay node providers for this service when we buy ICP off of the exchange. IMO, the IC is very vulnerable right now. The demand for NFTs (across all chains) has reduced significantly and we don’t have DeFi to bring in money flows. Many of our other services are designed to be free to users. This is great for adoption, but it does not pay the bill (node providers). We need to consider that there are other risks associated with this delay and factor those into our decision as well.

Sorry for getting on a soapbox. I was actually supposed to take this week off from the IC. Unfortunately, between the IC Gallery Moonwalkers launch (I love utility/access NFTs) and this discussion (I work as a security engineer) I couldn’t help myself.

10 Likes

I consider this part of the preparation work for the deployment of the proposal, so I see nothing wrong. I am sure there are many other things that the developers resolve as they plan to deploy, and hence the time delay between the vote and the deployment, and this is no exception. We will still get to hold ICP in canisters as we voted.

3 Likes

Great points. The caveat “if we find, upon further investigations, that such risks become unmanageable, then we will…” should fix this for future proposals.

5 Likes

Wrt congestion there are several concerns that can be addressed individually. To some extent meeting the ETA is a question of reaching the right tradeoff, we will not aim for perfection but tackle the highest priority ones and ask for an ok from the security team. Since new findings have trickled in and the resolution features have just been created the ETAs are not yet established so while still optimistic about January I have to acknowledge the risk of delay.

5 Likes

Since we’re looking at a much later release date, I want to give an update of where we’re at.

Implementation
The change wrt allowing canisters to transfer ICP is small and we have the enablement of canisters to transfer ICP ready and tested. Accompanying it is a bigger piece of work, a candid interface to specify how to interact with the ledger.

Documentation
We have written a spec for the ledger canister. We are working on developer guidance on how to transfer ICP in canisters and also guidance on how to verify canister smart contracts. As part of verifying a canister smart contract you may want to check that the canister’s Wasm code is correct, and we have already published guidance to verify it matches original source code.

Security
I’m going to be deliberately vague wrt the concerns but let me try to give the gist.
One area I’ve called congestion. This relates to the concern that there can be delays in transferring ICP, an unfortunate outcome could for instance be you send an ICP to fulfill a smart contract obligation but it arrives too late because an attack has clogged the system. Congestion is an umbrella term, there are several things we want to improve, some of them found recently. We currently have one feature addressing the most important of these issues with ETA at the end of November, other features are in the works.
Another area is sandboxing. We are not aware of concrete attacks but are concerned about the risk of Wasm jailbreak. We have increased the priority of canister sandboxing to address this issue, with ETA at the end of December.

7 Likes

Thanks for the update.

I believe the congestion mitigation and sandbox features will also help improve security for canister principals holding arbitrary tokens (e.g. SNS tokens once they are available).

At the same time, I agree with @LightningLad91’s general message. A single wasm jailbreak isn’t enough—an attacker would need to hijack 2/3 of the nodes in that subnet to override consensus. That doesn’t seem easy.

Security is important, but so is execution. Personally, I would be in favor of implementing the proposal now, and not being blocked on congestion mitigation and sandboxing. They can continue to be worked on in the background.

4 Likes

Hey folks,

Here is my follow-up to the quote above, what do all you think? Below is draft NNS motion proposal to bless the change to the plan.

Postponing proposal 20588 (Enabling canisters to hold ICP) for security concerns

1. Objective

The engineers and researchers working on the project “enabling canisters to hold ICP” discovered some security risks and issues that lead them to delay this project until some security issues are resolved (ETA: end of December 2021).

2. Background

September 17, 2021: NNS motion proposal #20588 to work on “enabling canisters to hold ICP” passed. This is an important proposal and feature as it can accelerate the growth of defi on the Internet Computer.

October 22, 2021: the team working on the project announced that they had discovered some security issues which their better judgment leads them to address first before enabling canisters to hold ICP.

October 26, 2021: After discussing in the developer forum, the consensus was the project should not be delayed unilaterally by the security concerns, but rather that the community vote and bless the delay. Enable Canisters to Hold ICP - #129 by diegop

3. State of the project currently

Implementation

The change with respect to allowing canisters to transfer ICP is small and the team working on it has the enablement of canisters to transfer ICP ready and tested. Accompanying it is a bigger piece of work, a Candid interface to specify how to interact with the ledger.

Documentation

The team has has written a spec for the ledger canister. They are working on developer guidance on how to transfer ICP in canisters and also guidance on how to verify canister smart contracts. As part of verifying a canister smart contract one may want to check that the canister’s Wasm code is correct, so they have already published guidance to verify it matches original source code 3.

Security

The team is “deliberately vague” with respect to the security
concerns, but the gist has two areas:

a. “Congestion” - This relates to the concern that there can be delays in transferring ICP, an unfortunate outcome could for instance be that one sends an ICP to fulfill a smart contract obligation but it arrives too late because an attack has clogged the system. Congestion is an umbrella term, there are several things the team wants to improve, some of them found recently. They currently have one feature addressing the most important of these issues with ETA at the end of November, other features are in the works.

b. “Sandboxing” - The team is not aware of concrete attacks but are concerned about the risk of Wasm jailbreak. We have increased the priority of canister sandboxing to address this issue, with ETA at the end of December.

8 Likes

I think it would be worthwhile to include this statement from @JensGroth as well

1 Like

What I still don’t understand is why we can’t enable “canisters to hold ICPs” while explaining people it’s beta (like the whole IC is)?

Or, put differently: Isn’t there a cognitive dissonance between “we need to prevent people from putting their ICPs at risk, and hence put up technical hurdles” and, at the same time, “we applaud people to already run valuable NFT and other valuable projects on the IC”? Why doesn’t DFINITY put technical hurdles in place to prevent, say, valuable NFTs on the IC (e.g. resetting all canister state)?

I’d be strongly in favor of removing the restriction, together with clear communication about the risk (just like we clearly say that the whole IC is still in beta). This way, development of dApps that involve ICPs can at least begin, and the ecosystem will be ready once the IC is ready!

Is this (alternative) option going to be part of the vote? (Or is rejecting your postponing proposal exactly that?)

10 Likes

I agree.

I find it odd that way the proposal is written. We are voting to “adopt” a proposal to delay. And if we want to implement the original proposal we must “reject” this one.

Wouldn’t it be better to state the updated security concerns, include the roadmap for implementing fixes, and then ask the community to “adopt” a proposal to accept the risk and proceed, or “reject” the proposal (resulting in a delay)?

4 Likes

We were thinking of the approach to launch with warnings that the IC is in beta. The risk is that regardless users use big amounts. Of course we can wash our hands if there is an attack and say, hey, we warned you it is in beta. But it would still be sad for those users and it would still give bad publicity for the IC.
Having said that, I would indeed interpret a reject vote as ‘go ahead regardless of sandboxing or not as we originally planned’. I’d also consider a rejection of the delay as increasing the risk appetite wrt congestion.

2 Likes

Which is what we would do if the NFT canisters get attacked, right? It seems equally bad for our users … but maybe not equally bad for the reputation. So a sarcastic conclusion is that the motivation isn’t the users safety but dfinity’s reputation - which I guess is fair and reasonable, in an unfortunate way.

3 Likes

There’s a lot I want to say but I honestly don’t think it matters.

Fwiw (coming from a user), I’ll be voting against this proposal.

1 Like

I am not sure I follow the latest few comments, so I will lay out what I see so folks can help me piece them together:

  1. @nomeata asked why we cannot just go ahead with security implications and just warn people (very fair question). My answer (which I think @JensGroth would agree) is “*we absolutely could. The security team is conservative, but perhaps there is more community appetite for risk than the security team has.”

  2. @LightningLad91 will be voting against a proposal to delay this (again, totally fair). Should I interpret this statement meaning “I have the appetite for the risk of congestion and wasm jailbreak”? Or something more subtle I am over-simplifying?

Am i understanding correctly?

2 Likes

Not sure when the new proposal will go live, but I also will vote against it (meaning that we should let canisters hold ICP as soon as possible).

Basically, I think the risk is acceptable, and a lot of DeFi projects have been waiting for this (and I’m waiting for some of those DeFi projects).

2 Likes