Solidity Security Best Practices: What 25 Years of Code Review Taught Me
TL;DR: After 27 years of code review across every major programming language and hundreds of smart contract audits, the patterns are universal: trust nothing, validate everything, and assume attackers are smarter than you. Here’s what actually prevents exploits.
Security best practices lists are everywhere. Most of them are useless — generic advice that doesn’t reflect how real attacks happen. “Use SafeMath.” “Avoid reentrancy.” Great, but why? And more importantly, how do you actually implement these patterns in production code under deadline pressure?
This isn’t a checklist. This is what I’ve learned from reviewing code that protected billions (and code that lost millions). The patterns that actually work when attackers with $30M bounty incentives are hunting for vulnerabilities in your contracts.
1. Checks-Effects-Interactions (The Pattern That Prevents Billions in Losses)
Reentrancy has cost over $1 billion across hundreds of exploits. Cream Finance: $18M. Hundred Finance: $80M. The DAO: $60M (2016). It keeps happening because developers don’t follow this one simple pattern.
Wrong (vulnerable to reentrancy):
function withdraw(uint256 amount) external {
require(balances[msg.sender] >= amount, "Insufficient balance");
// DANGER: External call before state update
(bool success, ) = msg.sender.call{value: amount}("");
require(success, "Transfer failed");
balances[msg.sender] -= amount; // Too late
}
When you send ETH to msg.sender, if they’re a contract, their receive()` or fallback()` function executes. They can call `withdraw()` again before you've updated their balance. Recursive drain.
Right (secure):
function withdraw(uint256 amount) external {
// 1. CHECKS: Validate inputs and authorization
require(balances[msg.sender] >= amount, "Insufficient balance");
require(amount > 0, "Cannot withdraw zero");
// 2. EFFECTS: Update state FIRST
balances[msg.sender] -= amount;
totalDeposits -= amount;
// 3. INTERACTIONS: External calls LAST
(bool success, ) = msg.sender.call{value: amount}("");
require(success, "Transfer failed");
}
Now if they try to reenter, balances[msg.sender] is already zero. The second call fails the require check. Attack prevented.
This pattern is non-negotiable. Every time you make an external call (transfer, delegatecall, any interaction with another contract), state must be updated first.
2. ReentrancyGuard for Defense in Depth
Even with Checks-Effects-Interactions, add a reentrancy guard. Why? Because codebases evolve. Someone refactors six months from now and accidentally introduces a cross-function reentrancy. The guard catches it.
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract SecureLending is ReentrancyGuard {
function withdraw(uint256 amount) external nonReentrant {
// Your logic here
// If any function in the call stack has nonReentrant,
// reentering will automatically revert
}
function borrow(uint256 amount) external nonReentrant {
// Protected from cross-function reentrancy too
}
}
OpenZeppelin's ReentrancyGuard is battle-tested. Use it. Don't write your own unless you understand mutex locking at a deep level.
3. Integer Overflow/Underflow Protection
Solidity 0.8.0+ has built-in overflow checks. Use it. If you're on an older version for some reason, use SafeMath everywhere:
// Solidity >= 0.8.0: automatic overflow protection
uint256 result = a + b; // Reverts on overflow
// Solidity < 0.8.0: MUST use SafeMath
using SafeMath for uint256;
uint256 result = a.add(b); // Reverts on overflow
The BEC token hack (2018) happened because of unchecked multiplication. Attackers exploited amount * recipients.length overflow to mint unlimited tokens. The protocol went from $70M market cap to worthless overnight.
If you need unchecked math for gas optimization, be extremely explicit:
unchecked {
// Only use unchecked when you've mathematically proven
// overflow is impossible or intentional
uint256 i;
for (i = 0; i < 1000; i++) {
// Safe: i will never exceed 1000
}
}
4. Access Control That Actually Works
Most access control bugs come from one of three mistakes:
Mistake #1: Missing Access Control
// DANGER: Anyone can call this
function setInterestRate(uint256 newRate) external {
interestRate = newRate;
}
I see this constantly in audits. Critical functions with no access control. Use modifiers:
import "@openzeppelin/contracts/access/Ownable.sol";
contract SecureProtocol is Ownable {
function setInterestRate(uint256 newRate) external onlyOwner {
require(newRate <= MAX_RATE, "Rate too high");
interestRate = newRate;
}
}
Mistake #2: tx.origin Instead of msg.sender
// DANGER: Phishing attack vector
function withdraw() external {
require(tx.origin == owner, "Not owner");
// If owner calls a malicious contract, that contract can call this
// tx.origin will still be owner
}
Always use msg.sender for authorization. tx.origin is the original external account, which can be manipulated through intermediary contracts.
Mistake #3: Uninitialized Upgradeable Contracts
The Wormhole bug that was worth a $10M bounty: the implementation contract was deployed but never initialized. Anyone could call initialize(), become owner, and destroy the contract with selfdestruct.
// Upgradeable contract pattern
contract MyImplementation is Initializable {
address public owner;
function initialize(address _owner) public initializer {
owner = _owner;
}
}
// After deployment, IMMEDIATELY call initialize
// Or use a constructor that disables initializers:
constructor() {
_disableInitializers();
}
5. Oracle Manipulation Defenses
Flash loan attacks often exploit price oracle manipulation. Bonq DAO lost $100M when attackers manipulated the oracle to make 0.1 tokens worth $100M.
Vulnerable pattern (single-block TWAP):
// DANGER: Can be manipulated within one block
function getPrice() external view returns (uint256) {
return uniswapPair.price0CumulativeLast();
}
Secure pattern (time-weighted average):
uint256 public priceAccumulator;
uint256 public lastUpdateTime;
uint256 constant TWAP_PERIOD = 30 minutes;
function updatePrice() external {
uint256 currentPrice = uniswapPair.price0CumulativeLast();
uint256 timeElapsed = block.timestamp - lastUpdateTime;
require(timeElapsed >= TWAP_PERIOD, "Too soon");
// Average price over TWAP_PERIOD
uint256 avgPrice = (currentPrice - priceAccumulator) / timeElapsed;
priceAccumulator = currentPrice;
lastUpdateTime = block.timestamp;
}
Better still: use Chainlink oracles for critical price data. They aggregate multiple sources and have built-in manipulation resistance.
6. Safe External Calls
Every external call is a potential attack vector. Treat them like explosives.
// DANGER: Forwards all gas, returns false on failure
msg.sender.call{value: amount}("");
// BETTER: Check return value
(bool success, ) = msg.sender.call{value: amount}("");
require(success, "Transfer failed");
// BEST: Use address.transfer() for simple ETH sends (2300 gas limit)
// This prevents reentrancy but will fail with complex recipient contracts
payable(msg.sender).transfer(amount);
// OR: Pull payment pattern - let users withdraw themselves
pendingWithdrawals[msg.sender] += amount;
// User calls separate withdraw() function
For token transfers, always use SafeERC20:
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
using SafeERC20 for IERC20;
// Handles tokens that don't return bool properly
token.safeTransfer(recipient, amount);
token.safeTransferFrom(sender, recipient, amount);
7. Validate All Inputs (Even "Trusted" Ones)
After 27 years of code review, I promise you: every input will eventually be attacker-controlled. Plan accordingly.
function deposit(address token, uint256 amount) external {
// Validate token address
require(token != address(0), "Zero address");
require(supportedTokens[token], "Token not supported");
// Validate amount
require(amount > 0, "Cannot deposit zero");
require(amount <= MAX_DEPOSIT, "Amount too large");
// Validate msg.sender isn't a contract (if you want EOA-only)
require(msg.sender == tx.origin, "No contracts");
// Or validate msg.sender IS approved
require(approvedUsers[msg.sender], "Not approved");
// Proceed with validated inputs
}
Yes, this costs gas. It also prevents exploits. Choose wisely.
8. Immutable Critical Variables
If a variable should never change after deployment, make it immutable. This prevents upgrade attacks and admin key compromises from modifying core parameters.
contract LendingPool {
// Can be set once in constructor, then never changed
address public immutable collateralToken;
address public immutable lendingToken;
uint256 public immutable maxLTV; // Loan-to-value ratio
constructor(address _collateral, address _lending, uint256 _maxLTV) {
collateralToken = _collateral;
lendingToken = _lending;
maxLTV = _maxLTV;
}
// No function can ever change these values
// Even if admin keys are compromised
}
9. Emergency Pause Mechanism
When Babylon Finance was under attack (we faced APT-level sophistication), having an emergency pause saved us. Not all protocols have this luxury, but if yours does, implement it correctly:
import "@openzeppelin/contracts/security/Pausable.sol";
contract SecureProtocol is Pausable, Ownable {
function deposit() external whenNotPaused {
// Normal operation
}
function withdraw() external whenNotPaused {
// Normal operation
}
// Emergency function - owner only
function emergencyPause() external onlyOwner {
_pause();
}
// Always allow users to withdraw even when paused
function emergencyWithdraw() external {
// No whenNotPaused modifier
// Users can always get their funds out
}
}
Critical: emergency pause should NEVER prevent users from withdrawing their funds. Only block new deposits and protocol operations.
10. Testing Like You Mean It
This isn't a best practice, it's the foundation everything else rests on:
- Unit tests: Every function, every edge case
- Integration tests: Functions working together
- Fuzzing: 10,000+ random inputs (see my fuzzing article)
- Mainnet forking: Test against real state with Foundry
- Formal verification: For critical math and invariants
Target: >95% code coverage. Anything less and you're gambling with user funds.
# My standard testing workflow
forge test --gas-report
forge test --fuzz-runs 10000
forge coverage
forge test --fork-url $ETH_RPC_URL --fork-block-number 17000000
What This All Comes Down To
Security isn't a checklist. It's a mindset. After reviewing hundreds of audits and watching billions get stolen, here's what separates secure protocols from hacked ones:
- Secure protocols assume attackers are sophisticated, well-funded, and persistent. They are.
- Secure protocols have defense in depth. Multiple overlapping protections, not single points of failure.
- Secure protocols are simple. Complexity is where bugs hide. Every line of unnecessary code is risk.
- Secure protocols get audited. Multiple times. By different teams. And they fix what auditors find.
- Secure protocols have bug bounties. Pay $100K to a white hat now or $10M to a black hat later. Easy choice.
I've spent 27 years in security. Web1, web2, mobile, IoT, industrial control systems, and now blockchain. The technologies change. The fundamentals don't.
Trust nothing. Validate everything. Assume breach. Plan for failure. Test relentlessly. And when you think you're done, test again.
Because in DeFi, the only second chance you get is your next protocol.
Stay safe out there.
Want deeper dives into blockchain security? Visit blockchainwhitehackers.com
Disclaimer: This article was researched and written by members of BWH Academy, with AI-assisted research and drafting. While we strive for accuracy, details may slightly differ from exact real-world scenarios. All content is provided for educational and learning purposes only — not as professional security advice.
No Comments