We have deployed a new version of our sUDT and anyone-can-pay (acp) lock contract on Nervos CKB mainnet. The contracts were audited by Trail of Bits and the full auditing report can be found here.
Some of you might have already seen a notice requesting upgrades on the acp smart contract, either in Neuron (version 0.33.2 or later) or in Bitpie wallet (version 5.0.009 or later). The assets in the previous version of the acp lock contract are safe, users can have their assets migrated to the new acp lock contract within their wallets.
For developers, all they need to do is to parse the acp short address with the new code hash, and introduce a portal for users to migrate their assets, if necessary.
Trail of Bits Audit Collaboration
Since October 2020, we have conducted several rounds of security audits for the sUDT contract, as well as anyone-can-pay lock contract. The security experts at Trail of Bits have done an amazing job at auditing the smart contracts, and helped us identify several issues that existed in our implementations. While most of the issues do not affect the result binary, two vulnerabilities stood out, and we had to patch and release a newer version of the anyone-can-pay smart contract.
For more details on the audit, please refer to the full report here. Internally, we have carefully discussed and resolved each individual issue occurred in the audit report:
Docker-based contract build process depends on molecules in PATH
This has been resolved in this commit. Luckily, this does not affect the final binary, and our build process does have checkings, ensuring the correct and unique binary is built. However, it still helps us refine the process to build more secure smart contracts in the future.
Contracts use an out-of-date ckb-c-stdlib dependency
To us, this is more of a design choice made in CKB smart contract development. For now, smart contracts are all statically linked, meaning that a smart contract must pack all the dependent libraries within its own binary. Once a smart contract is released to mainnet, all its dependent libraries will be fixed, there is no way we can change those libraries without resulting in a new binary.
Due to this design, each deployed smart contract will be locked to a particular version of the ckb-c-stdlib library. Even when we add new features in the future, we cannot easily upgrade to a newer version of ckb-c-stdlib. Binary stability, as well as reproducible build, is a very important foundation for CKB to be sure.
That does not mean we are not aware of the problem. Our strategy here is that we will treat the particular version of ckb-c-stdlib library as part of the smart contract. When some vulnerabilities are exposed in the library, we will issue a security fix, and update the binary as well. This way, we can make sure even though older libraries are used, they can still be sure enough for on-chain usage.
Another potential solution to this problem is dynamic linking techniques on CKB that are being developed by the Nervos Foundation. Once dynamic linking is matured, we can dynamically link dependent libraries in CKB-VM. This way when a vulnerability is discovered in the library, only the particular library can be upgraded — the on-chain binaries can stay unchanged.
GCC 9.2 through 10.2 miscompile certain uses of memcmp
This is more of an information note. Roughly in the same timeline as Trail of Bits is conducting on this review we became aware of a GCC bug which might affect secp256k1 library used in many CKB smart contracts. So, we consulted the security experts at Trail of Bits on this issue. It turns out that the GCC version used in CKB is not affected by this issue. No further action will then be needed on our side. Still, we plan to keep a closed eye on the issue in case more findings surface.
Implementation of sbrk does not set errno on failure
This is indeed a problem in our code since we didn’t set errno as standard sbrk calls. However, this won’t affect our smart contracts here since none of the smart contracts used in CKB so far uses mallocs; hence no sbrk calls will be issued. Still, we plan to fix this issue soon and release a new version of CKB RISC-V toolchain that does not contain this bug.
Uninitialized variables are read
This is one of the two vulnerabilities that must be fixed on our side. A fix for this bug can be found here.
Undefined behavior for CKB-only cells
This is another vulnerability that must be fixed on our side. A fix for this bug can be found here.
Duplicated logic in the Anyone Can Pay contract
This is something that we overlooked while building the smart contract. While it is not a vulnerability by itself, we also fixed it together with the previous two vulnerabilities. A fix for this bug can be found here.
The mbedtls library is built in non-production mode
This was discovered since we have RSA related code in the repository containing the above two smart contracts, but as the audit report already points out, RSA contracts, back at the time, were still being developed and not in a production ready state. We totally appreciate Trail of Bits for pointing this out and we have resolved this issue in our code. We want to point out that the RSA smart contract will have its own round of security audits, only after that will we advise production usage of the RSA smart contract.
nervosnetwork/riscv-newlib is 700 commits out of date
This is also something that is due to the design of CKB smart contracts, which is similar to the ckb-c-stdlib library note above. Due to reproducible build concerns, we cannot simply upgrade the dependencies used in a smart contract after the smart contract has been released. So, at Nervos, we are enforcing a different practice: we will upgrade the RISC-V toolchain at a much slower pace. Right now the RISC-V toolchain we provided has been carefully tested so we can be sure there is little chance of bugs or vulnerabilities in them. We plan to upgrade the toolchain to a newer version of GCC as well as the newlib library. But we do want to mention that this takes time and careful testing on our side. Also, when a vulnerability occurs in the older version of the toolchain, we will make sure to patch the toolchain with a fix and upgrade affected smart contracts to a newer version as well.
Upgrading smart contract in the future
The smart contract upgradability is very important and we want to make sure there is a smooth upgrading path without affecting users. This time, we will have to let each affected users manually upgrade to a newer version, but we do want to prevent hassles like this in the future.
Given this consideration we plan to introduce *upgradability for smart contracts* deployed by the CKB core team (cell capacity provided by the Nervos foundation). The smart contract upgradability works as follows:
* When a smart contract is first deployed on ckb, a special mechanism named Type ID will be provided so each smart contract can get a unique name. A multisig lock is used for the cell containing the smart contract, and the multisig lock is maintained by a system contract upgrade committee formed by members from CKB core team and Nervos foundation for now. The multisig is set of 3 of 5, meaning as long as 3 of those 5 provide signatures, the smart contract can be upgraded.
* When using the smart contract, one should not use the data hash directly but leverage Type ID techniques.
* When a vulnerability is discovered, the smart contract containing cell can be unlocked by the committee and upgraded to a new version with a bug fix. Users who reference the smart contract by Type ID will use the new version without any manual operations. This way, vulnerabilities can be handled in a seamless fashion.
* After a certain amount of time, when the committee has enough confidence that a smart contract is secure enough, the committee can finalize the smart contract by upgrading the multsig lock for its containing cell to an all-zero lock, so no one will be able to alter it afterwards. The upgrade process can be further decentralized with community governance in the future. By replacing the multi-sig lock with a Governance Lock, the smart contract containing cell can only be unlocked under certain governance results.
* Furthermore, if a user did not want the lock to be upgraded automatically by committee or governance, the user can still choose to use the sUDT and ACP lock with code hash instead of hash type. It will not be affected by any upgrade, and no one can change the user’s contract which will continue to use the code specified when the cell was created, not the new code after the upgrade.
We hope by leaving both options to users, a conflict between seamless vulnerability fixes and the “code is law” philosophy can be balanced. A better user experience, as well as a decentralized future, can be achieved at the same time.
To stay updated on all things Nervos:
Join our community: Discord — Github — Nervos Talk Forum — Twitter
For discussions or questions join the conversation on Discord or check out one of our community Telegram channels: English, Korean, Russian, Japanese, Spanish, Vietnamese and Chinese