Solidity Security Best Practices: What 25 Years of Code Review Taught Me

Blockchain White Hackers  Security Audits   Solidity Security Best Practices: What 25 Years of Code Review Taught Me

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
Leave a Comment:
Skip to main content