The Ultimate Guide to Auditing a Smart Contract + Most Dangerous Attacks in Solidity
Ever wondered how to audit a smart contract to find security breaches?
You can learn it yourself, or you can use this helpful step-by-step guide to learn exactly what to do, when and when to audit these contracts.
I've been researching various Smart Contract audits and learned the most common steps they take to extract all essential information from any contract.
You will learn the following:
Steps to take to fully audit a Smart Contract to generate a pdf with all conclusions.
The most important types of attacks you need to know about as an Ethereum Smart Contract Auditor.
What to look for in a contract and helpful tips you won't find anywhere else but here.
Let's cut to the chase and start auditing contracts:
How to audit a Smart Contract
To teach you exactly how to do this, I'm going to audit one of my own contracts. This way you will see a real world audit that you can apply for yourself.
Now you might ask, what exactly is a Smart Contract audit?
A Smart Contract audit is the process of carefully investigating a piece of code, in this case a Solidity contract for bugs, vulnerabilities and risks, before the code is deployed and used on the Ethereum mainnet where it will not be modifiable. It's just for discussion purposes.
Note that an audit is not a legal document that verifies that code is secure. Nobody can guarantee 100% that the code will not have future bugs or vulnerabilities. It's a guarantee that your code has been reviewed by an expert and is secure.
To discuss possible improvements and mainly to find bugs and vulnerabilities that might risk people's Ether .
Once that's clear, let's take a look at the structure of a Smart Contract Audit:
Disclaimer : Here you will say that the audit is not a legally binding document and that it does not guarantee anything. That this is just a discussion paper.
Audit overview and legal features : A quick overview of the Smart Contract that will be audited and best practices found.
Attacks made on the contract : In this section you will talk about the attacks made on the contract and the results. Just to verify that it is, in fact, safe.
Critical vulnerabilities found in the contract : Critical issues that could seriously undermine the integrity of the contract. Some mistakes that would allow attackers to steal Ether is a critical issue.
Medium vulnerabilities found in the contract : those vulnerabilities that could damage the contract, but with some kind of limitation. Like a bug that allows people to modify a random variable.
Low severity vulnerabilities found : These are the issues that really don't break the contract and that could exist in the deployed version of the contract.
Line-by-line comments : In this section, you'll review the most important lines where you see potential improvements.
Audit Summary : Your view of the contract and final audit findings.
Keep this structure somewhere safe because it's all you need to actually securely audit a Smart Contract. It will really help you find those hard to find vulnerabilities.
I recommend that you start with point 7 "Line-by-line comments" because by analyzing the contract line-by-line you will find the most important issues and see what is missing. What could be changed or improved.
I'll show you a Disclaimer that you can use like this for the first step of the audit. You can go to point 1 and down from there until the audit is complete.
Then I'll show you my personal audit that I did for one of my contracts using that framework with these steps. You will also see a description of the most important attacks that can be made on a Smart Contract in step 3.
In this Smart Contract audit, we cover the following topics:
Audit overview and nice features
Attack made on the contract
Critical vulnerabilities found in the contract
Average vulnerabilities found in the contract
Low severity vulnerabilities found
The audit makes no representations or warranties about the usefulness of the code, security of the code, suitability of the business model, regulatory regime of the business model, or any other representations about the contracts' fitness for purpose, or their error-free status. Audit documentation is for discussion purposes only.
The project has only one file, the file Casino.sol, which contains 142 lines of Solidity code. All functions and state variables are well commented using the natspec documentation for the functions, which is good for quickly understanding how everything should work.
The project implements the Oraclize API to generate truly random numbers on the blockchain using a centralized service.
Generating random numbers on the blockchain is a rather difficult topic because one of the core values of Ethereum is predictability, the aim of which is not to have undefined values.
Therefore, using Oraclize 's reliable number generation is considered good practice, as they generate random numbers off the blockchain . It implements modifiers and a callback function that verifies that the information comes from a trusted entity.
The purpose of this Smart Contract is to participate in a random lottery where people bet on a number between 1 and 9. When 10 people place their bets, the prize is automatically distributed among the winners. There is also a minimum bet amount for each user.
Each player can only bet once during each game and the winning number is only generated when the betting limit is reached.
The contract offers a good set of functionalities that will be useful for the whole contract:
Secure random number generation with Oraclize and proof checking in the callback .
Modifiers to verify the end game, blocking critical functions until rewards are distributed.
A fair amount of checking to verify that the wager function is used correctly.
Secure generation of the winning number only when the maximum bets have been reached.
3. Attacks made on the contract
In order to verify the security of the contract, we test various attacks to ensure that the contract is secure and follows best practices.
This attack consists of recursively calling the method call.value()on an ERC20 token to extract the ether stored in the contract if the user is not updating the balancesender's before sending the ether.
When you call a function to send the ether to a contract, you can use the fallback function to rerun that function until the ether from the contract is extracted.
As this contract uses transfer()instead of call.value(), there is no risk of reentrancy attacks since the transfer function only allows you to use 23,000 gas which you can only use for one event to log data and cast on failure.
That way you won't be able to call the sender function again, thus avoiding the reentrancy attack.
The transfer function is called only when distributing rewards to winners, which happens once per game, when the game ends. So there shouldn't be any problem with reentrance attacks.
Note that the condition for calling this function is that the number of bets is greater than or equal to the 10 bet limit, but this condition is not updated until the end of the function distributePrizes()which is risky because someone could theoretically be able to call this function and execute all the logic before updating the state.
So my recommendation is to update the condition when the function starts and set the number of bets to 0 to avoid calling the function distributePrizes()more times than anticipated.
An overflow happens when the limit of the type variable uint256, 2**256, is exceeded. What happens is that the value is returned to zero instead of increasing further.
For example, if I want to assign a value to a uint greater than 2**256, it will simply go to 0 - this is dangerous.
On the other hand, an underflow happens when you try to subtract a number greater than 0 from 0.
For example, if you subtract 0 -1, the result will be = 2**256 instead of -1.
This is quite dangerous when it comes to ether . However, in this contract there is no subtraction anywhere, so there is no risk of underflow .
The only time an overflow can happen is when bet()(betting) a number and the amount of the variable TotalBetis increased:
Someone could send a huge amount of ether that would exceed the 2**256 limit and therefore make the total bet 0. This is unlikely, but the risk is there.
Therefore, I recommend using a library such as OpenZeppelin's SafeMath.sol.
It will help you make safe calculations without the risk of under or overflow .
The way you use it is by importing the library, activating it for uint256 and then using the .mul(), .add(), sub() and .div() functions. For example, the .mul(), .add(), sub() and .div() function:
using SafeMath for uint256;
function example(uint256 _value) {
uint number = msg.value.add(_value);
The replay attack consists of making a transaction on a blockchain like the original Ethereum blockchain and then replaying it on another blockchain like the classic Ethereum blockchain .
Ether is transferred as a normal transaction from one blockchain to another.
Though it's not a problem anymore because since version 1.5.3 of Geth and 1.4.4 of Parity both implement Vitalik Buterin's EIP 155 attack protection
Therefore, the people who will use the contract are dependent on their own ability to stay current with these programs to stay safe.
This attack is where a miner or other party tries to "compete" with a participant in a Smart Contract by entering their own information into a list or mapping so that the attacker can get lucky in getting their own information stored in the contract.
When a user enters his bet()and the data is saved on the blockchain , anyone will be able to see which number has been wagered, simply by calling the mapping playerplayerBetsNumber .
This mapping shows which number was selected by each person. So in the transaction data you can easily see the amount of ether that was staked.
This can happen in the function distributePrizes()because it is called when the callbackrandom number generation is invoked.
Since the condition of this function is not updated until the end, there is a risk of a reordering attack.
Consequently, my recommendation is as I said before: update the number of bets condition at the start of the function distributePrizes()to avoid this kind of unforeseen behavior.
This attack affects ERC20 tokens , it was discovered by the Golem team and consists of the following:
A user creates an ethereum wallet with a traling 0 , which is not difficult because it is just a single digit. For example: 0xiofa8d97756as7df5sd8f75g8675ds8gsdg0
Then he buys tokens , removing the last zero:
Buy 1000 tokens from 0xiofa8d97756as7df5sd8f75g8675ds8gsdg account
If the token contract has enough amount of tokens and the purchase function does not check the sender address length, the Ethereum virtual machine will just add zeros to the transaction until the address is complete.
The virtual machine will return 256000 for every 1000 tokens purchased. This is a virtual machine bug that hasn't been fixed yet, so whenever you want to buy tokens , make sure you check the address length.
The contract is not vulnerable to this attack as it is not an ERC20 token.
4. Critical vulnerabilities found in the contract
There are no critical issues in the audited smart contract.
5. Average vulnerabilities found in the contract
The function checkPlayerExists()is not constant when it should be.
Therefore, this increases gas costs each time the function is called, which is a big problem when dealing with many calls.
Make this constant and avoid expensive gas runs .
6. Low severity vulnerabilities found
You are using assert()instead of require()in all cases and at the beginning of the functions ` call back()` and pay().
Assert and require behave almost identically, but the assert function is used to validate the state of the contract after making changes, while require is usually used on top of functions to verify function input.
You are defining the variable players at the beginning of the contract, but not using it anywhere. Remove it if you are not going to use it.
Line 1 : You are specifying a pragma version with the caret symbol (^) in front, which tells the compiler to use any version of solidity greater than 0,4,11.
This is not a good practice as there could be big changes between versions that would make your code unstable. That's why I recommend setting a fixed version without the accent to 0.4.11.
Line 14 : You are defining the uintvariable totalBetin the singular, which is not correct as it stores the sum of all bets. My recommendation is to change it to plural, totalBets instead of totalBet .
Line 24 : You are defining the constant variable in caps which is good practice to know that it is a fixed, unmodified variable.
Line 30 : As I said before, you are defining an unused array . playerTake it out if you are not going to use it.
Line 60 : The function checkPlayerExists()should be constant, but it's not. Because it doesn't modify the state of the contract, it makes it constant and saves some gas every time it runs.
It's also good practice to specify the type of visibility the role has even if it's the default audience value to avoid confusion. To do this, explicitly add the public visibility parameter to the function.
Line 61 : You are not checking that the player parameter is sent and well formatted. Be sure to use a require(player != endereço(0));at the top of this function to check whether an invalid address exists or not. Also check the address length to protect the code against short address attacks, just in case.
Line 69 : Again, specify the function's visibility bet()to avoid confusion and know exactly what it should be called.
Line 72 : Use require()instead of assert()to check that the function input is well-formed.
Likewise, at the beginning of functions, require() is most often used. Change all assert() at the beginning to require() .
Line 90 : You are using a simple sum on the variable msg.value. This could lead to overflows , as the value could get quite large. That's why I recommend checking for overflows and underflows whenever you're doing a calculation.
Line 98 : The function generateNumberWinner()must be built-in, as you don't want anyone running it outside of the contract.
Line 103 : You are saving the result of oraclize_newRandomDSQuery()into a bytes32 variable . It is not necessary to execute the callback function . Also, you are not using this variable anywhere. Therefore, I recommend not assigning this value and just calling the function.
Line 110 : The function ____callback()_must be external because you only want it to be called from outside.
Line 117 : This claim must be required for the reasons I explained above.
Line 119 : You are using shae()which is not good practice as the algorithm used is not exactly shae3 , but keccak256 . My recommendation is to change it to keccak256() instead, for clarity.
Line 125 : The function distributePrizes()must be built-in because only the contract should be able to call it.
Line 129 : Even though you're using a variable-sized array for a loop , it's not too bad because the amount of winners should be limited to less than 100.
In general, the code is well commented and clear about what it should do for each function.
The mechanism for betting and distributing rewards is quite simple, so it shouldn't pose any major problems.
My final recommendation would be to pay more attention to the visibility of functions, as it is very important to define who should execute the functions and to follow best practices regarding the use of assert , require and keccak .
This is a secure contract that will safely store funds while it is working.
That was all the auditing I did myself using the framework explained at the beginning. I hope you've learned something and are now able to securely audit other Smart Contracts.
Keep learning and improving your knowledge of contract security, best practices, and new functionality.
