The Smart Contract Weakness Classification and Test Cases (SWC) Registry is a set of Web3 vulnerabilities to avoid when writing smart contracts. It may seem daunting to understand every issue so I’ll do my best to demystify every issue and explain each vulnerability with real-world examples. This write-up is intended for those who are just starting out in solidity development and smart contract auditing.
SWC-105: Unprotected Ether Withdrawal
This vulnerability pertains to insufficient access control (SWC-100) which may result in Ether being drained from the contract. This write-up will look at different types of insufficient access control.
As we go along with this SWC series, I will be constantly referring to previous SWC vulnerabilities to consolidate our understanding of what it means to be a smart contract auditor. Be sure to know every vulnerability by heart! These are the five bullet points that I will be covering today
Issue Breakdown
What is a modifier?
Reformatting the Issue
Bonus Practice
Summary
Issue Breakdown
Take a look at this contract. What is the problem with the code?
Let’s read through the contract line by line.
Line 1: This is the version used in the contract. Remember SWC 101, 102, and 103? Ideally, the version should be updated to 0.8.0 (SWC-102) at least to prevent any reentrancy errors (SWC-101). The caret symbol (SWC-103) should also be deleted to prevent any compiler errors from new version releases.
Line 5: This is the function called withdrawAllAnyone(). There is no function visibility (no public/external/internal/private modifiers), so the default visibility will be public (SWC-100). As the default visibility is public, this function can be called by anyone.
Line 6: The function withdrawAllAnyone() attempts to transfer the balance from the contract to msg.sender (the person calling the function). How do I know this? Let’s look at the transfer function in detail. When you see the word transfer, it takes in two parameters: the address you want to transfer to, and the amount you want to transfer. It looks something like this:
transfer(address to transfer to, how much you want to transfer)
transfer(msg.sender, this.balance)
Sometimes, the address you want to transfer to is written before the transfer itself, leaving the transfer with only one parameter (both semantics are allowed)
msg.sender.transfer(this.balance)
The word this refers to the contract. The word balance refers to the amount of ETH the contract has.
Remember, a contract can also hold tokens. If there is a transfer function, it is always attempting to transfer money out of the contract. If we are talking about sending money from one person to another, then we will use the transferFrom function (more on this in the future).
Line 9: This function has no name, but has a visibility modifier of public and a payable keyword, which means ETH can be transacted. When a function has no name, it is called a fallback function. A fallback function is an unnamed external function without any input or output parameters. EVM executes the fallback function on a contract if none of the other functions match the intended function calls.
For example, the fallback function is automatically executed when a transaction is sent to a contract without specifying any function to call. This means that if you send Ether to a contract address without specifying a function, the fallback function will be called by default (which may lead to some nasty vulnerability issues, which will be covered in SWC-107).
If you type this fallback code in Remix (an Ethereum Integrated Development Environment to test contracts out), it will return the following error:
“Expected a state variable declaration. If you intended this as a fallback function or a function to handle plain ether transactions, use the "fallback" keyword or the "receive" keyword instead.”
This function is written wrongly. Instead, it should be marked as external. Also, the keyword should be fallback or receive instead of ‘function’ for best practice.
(More about fallbacks in SWC-107)
Anyways, back to the issue. The problem is line 5, as anyone can call the function and withdraw the balance of the contract. This function is unprotected (it should have a modifier)
What is a modifier?
A modifier is code that can be run before and/or after a function call. Modifiers are used to restrict access, validate inputs, and guard against reentrancy (SWC-107).
When the contract is deployed, the constructor will run owner = msg.sender. Whoever deploys the contract will be the owner. Now, if I deployed the contract and became the owner, and I want to change owners, how should I do it properly? The answer is to use a modifier.
How do I write a modifier?
Firstly, use the modifier keyword and write a name for the modifier, in this case, modifier onlyOwner(). This modifier is extra code that will run before the main block of code in the function. The modifier onlyOwner() has one line of code which checks whether the msg.sender is the owner. At the end of the modifier, remember to write an underscore with a semi-colon. This means that the code in a function will continue to execute after the modifier.
Now, take a look at the bottom of the code, the function changeOwner has public visibility and two modifiers before the code in the actual function. This means that the function changeOwner will run the require statement in the onlyOwner and validAddress modifiers before running owner = _newOwner;
In advanced solidity, we can import OpenZepellin’s Ownable library so that we don’t need to write a modifier by ourselves. If you don’t understand this sentence, it’s perfectly alright, but all you need to know is that we can import modifier code that is written by someone else to save some time writing it by ourselves.
Reformatting the Issue
Going back to the issue in point one. Let’s say that only the owner can withdraw the contract balance. How should we reformat the issue to prevent anyone from withdrawing the contract’s balance? That’s right, we use a modifier. Here is the updated code with all the relevant changes.
No more floating pragma
Updated solidity version above 0.8
Added a modifier onlyOnwer
Function withdrawAll() has an onlyOwner modifier
Fallback function is external instead of public
Nice! This SWC vulnerability is good as a first pass for low-risk or non-critical issues in smart contract auditing. If you are a smart contract auditor, you can almost always refer to this issue and provide the necessary feedback when auditing your client’s code.
Bonus Practice
Wait a minute. Something’s fishy… Did you smell it?
That’s right! The onlyOnwer() modifier in the code above is spelled wrongly! The code above is still vulnerable! Remember to check the spelling and double-check every word! This is the hallmark of a good smart contract auditor.
The SWC-105 vulnerability is extremely broad. Not only does it cover unwarranted public functions, but spelling errors and wrongly written constructor name is also an issue! Let’s take a look at more examples. See if you can find the issue!
Hint: Take a look at the newOwner() function.
Hint: Take a look at the constructor or lack thereof.
Note: This is the version before 0.8.0, underflow is an issue
Hint: Look at the function withdraw()
Answers:
The newOwner() function does not have the onlyOwner modifier whereas the deleteOwner() has. Check whether the function uses the modifier it created
Function initWallet() can be called by anyone to become the new owner. It is supposed to be constructor initWallet() but it was erroneously written.
The comparison operator in the require statement of withdraw() is wrong. It should be require(amount <= balances[msg.sender]). In other words, I should only be able to withdraw an amount that is less than my current balance.
Summary
Check for unprotected Ether withdrawals
Check for spelling errors
Look at important functions and see if they can be accessed by anyone
That’s all for today’s write-up. Hope you learn something new and remember to look out for this SWC vulnerability in your code!