Tips on Making Your Smart Contracts Secure
As a smart contract developer, it is critical to have a defensive mindset where your code is the only thing that you can trust.
On this blog, I will share some tips that could make your smart contract more secure. All of these tips are based on existing vulnerabilities that have been exploited by hackers, which causes severe damage to the Blockchain community. Though knowing these tips will not make your smart contract immune to hackers, it can help you to avoid the most common bugs.
Hackers are everywhere
As a smart contract developer, it is critical to have a defensive mindset where your code is the only thing that you can trust.
Do not trust your users
When you are writing code, you should always assume that your users are hackers and will try their best to find vulnerabilities in your smart contract and definitely will exploit it without any hesitation. Therefore, you should never assume your users of using your smart contract honestly in the way that you expect, consider them as hackers instead. As a security researcher and a program developer, I got a lot of experiences of being attacked, what I learned from those is that no matter how small the flaw might look like, the consequence could be severe.
Write it down in your code
Writing a smart contract is like writing web applications; you are going to have a front end and back end. The smart contract that runs on the Blockchain is the back end. Besides the contract, you also need to write a front end application as an interface for your users to interact with your smart contract. And here comes the problem, should you verify the user input in the front end or the back end? For example, you are writing an alias service to link Blockchain address with human-readable string. Yet, you want to control the length of the text, where should you write down the format verification logic, front end or back end?
The answer is the back-end, for sure. Clique right? But it is more than essential. Sorry to say this, but you might also consider the front end developers as your enemies, they are on the other side with hackers. We smart contract developers write all the logics down in our code. And we only trust our code.
Length of the input
One big problem in Neo smart contract is the way it stores data. Storage.put() with a pair of key and value stores all kinds of data. It seems easy and cool to use, just like what we do while dealing with a dictionary or map. But the problem is that if you do not verify the key format properly while storing data, you are in big trouble. For example, we have a “totalSupply” keyword in our NEP5 smart contract indicating the maximum values of the nep5 coins. And we have transfer function in the contract:
public static bool Transfer(byte[] from, byte[] to, BigInteger amount)Currently, we have special length check logic in the NEP5 standard template and assume someone mistakenly removed such check logic. A user could pass any strings for the parameters:
//Check parameters
if (from.Length != 20 || to.Length != 20)throw new InvalidOperationException(“The parameters from and to SHOULD be 20-byte addresses.”);
Then, we pass the to parameter as “totalSupply”, in this way, we could change the value of “totalSupply” once the transaction executes successfully:
//Increase the payee balance
var toAmount = Storage.Get(context, to)?.AsBigInteger() ?? 0; Storage.Put(context, to, toAmount + amount);Therefore, please do check the format of input in every function. Neo transaction is
free, adding a few more lines of checking code won’t hurt.
I also think that as the most commonly used data, the address should have its name as a particular data type in the Neo smart contact.
Check the target address
Before I start taking about this issue, I’d like to ask you to take a look of the following transfer function from NEP5 smart contract:
public static bool transfer(byte[] from, byte[] to, BigInteger value)
{if (value <= 0 || to.Length != 20 || from.Length != 20) return false;BigInteger oldFromVal = Storage.Get(Storage.CurrentContext, from).AsBigInteger();if (oldFromVal < value) return false;BigInteger oldToVal = Storage.Get(Storage.CurrentContext, to).AsBigInteger();BigInteger newToVal = oldToVal + value;if (newToVal <= oldToVal) return false;BigInteger newFromVal = oldFromVal — value;if ((oldFromVal + oldToVal) != (newFromVal + newToVal)) return false;Storage.Put(Storage.CurrentContext, from, newFromVal); Storage.Put(Storage.CurrentContext, to, newToVal);return true;
}
This is not the original one; of course, I modified a little bit to make it vulnerable to a certain type of attack. Can you address the issue?
The bug could be triggered by passing from and to with the same address. With the same address assigned to from and to, the first put statement saves the newFromvalue to that address. Still, the second put statement changes that value to the newToVal, which is even bigger than its previous balance. In this way, you can mint new coins from thin air.
You think I am kidding? Just take a look here celer-smart-contract.
Author: Jinghui Liao
