Compound V2 Smart Contract Audit Report (Sample)

function distributeSupplierComp(address cToken, address supplier) internal {
    CompMarketState storage supplyState = compSupplyState[cToken];
    Double memory supplyIndex = Double({mantissa: supplyState.index});
    Double memory supplierIndex = Double({mantissa: compSupplierIndex[cToken][supplier]});
    compSupplierIndex[cToken][supplier] = supplyIndex.mantissa;
    // ❌ Index written BEFORE the zero-check — if supplier's stored index
    // ❌ was 0, the delta is now computed against the just-written current index,
    // ❌ not from compInitialIndex. The initialization branch fires too late.
    if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {
        supplierIndex.mantissa = compInitialIndex;
    }

    Double memory deltaIndex = sub_(supplyIndex, supplierIndex);
    uint supplierTokens = CToken(cToken).balanceOf(supplier);
    uint supplierDelta = mul_(supplierTokens, deltaIndex);
    // ❌ No cap — accrued amount can exceed total COMP in reservoir
    uint supplierAccrued = add_(compAccrued[supplier], supplierDelta);
    compAccrued[supplier] = supplierAccrued;
    emit DistributedSupplierComp(CToken(cToken), supplier, supplierDelta, supplyIndex.mantissa);
}

function distributeSupplierComp(address cToken, address supplier) internal {
    CompMarketState storage supplyState = compSupplyState[cToken];
    Double memory supplyIndex = Double({mantissa: supplyState.index});
    Double memory supplierIndex = Double({mantissa: compSupplierIndex[cToken][supplier]});

    // ✅ Initialization check runs BEFORE index is written — delta is now
    // ✅ computed from compInitialIndex for new suppliers, not from 0
    if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {
        supplierIndex.mantissa = compInitialIndex;
    }

    // ✅ Index written after initialization — ordering is correct
    compSupplierIndex[cToken][supplier] = supplyIndex.mantissa;

    Double memory deltaIndex = sub_(supplyIndex, supplierIndex);
    uint supplierTokens = CToken(cToken).balanceOf(supplier);
    uint supplierDelta = mul_(supplierTokens, deltaIndex);
    uint supplierAccrued = add_(compAccrued[supplier], supplierDelta);

    // ✅ Cap accrual to available reservoir balance — prevents distributing
    // ✅ more COMP than the contract holds
    uint reservoirBalance = EIP20Interface(getCompAddress()).balanceOf(address(this));
    compAccrued[supplier] = supplierAccrued > reservoirBalance
        ? reservoirBalance
        : supplierAccrued;

    emit DistributedSupplierComp(CToken(cToken), supplier, supplierDelta, supplyIndex.mantissa);
}

// GovernorBravo.sol — execute()
function execute(uint proposalId) external payable {
    require(state(proposalId) == ProposalState.Queued,
        "GovernorBravo::execute: proposal can only be executed if it is queued");
    Proposal storage proposal = proposals[proposalId];
    proposal.executed = true;
    // ❌ No guardian check or emergency pause mechanism — any queued proposal
    // ❌ executes after timelock delay with no additional safety gate
    for (uint i = 0; i < proposal.targets.length; i++) {
        timelock.executeTransaction{value: proposal.values[i]}(
            proposal.targets[i],
            proposal.values[i],
            proposal.signatures[i],
            proposal.calldatas[i],
            proposal.eta
        );
    }
    emit ProposalExecuted(proposalId);
}

// GovernorBravo.sol — execute() with guardian veto
function execute(uint proposalId) external payable {
    require(state(proposalId) == ProposalState.Queued,
        "GovernorBravo::execute: proposal can only be executed if it is queued");
    Proposal storage proposal = proposals[proposalId];
    // ✅ Guardian can veto high-risk proposals during the timelock window
    // ✅ without being able to propose or pass new changes unilaterally
    require(!proposal.vetoed, "GovernorBravo::execute: proposal vetoed by guardian");
    proposal.executed = true;
    for (uint i = 0; i < proposal.targets.length; i++) {
        timelock.executeTransaction{value: proposal.values[i]}(
            proposal.targets[i],
            proposal.values[i],
            proposal.signatures[i],
            proposal.calldatas[i],
            proposal.eta
        );
    }
    emit ProposalExecuted(proposalId);
}

// ✅ Guardian can veto — separate role from admin, limited to blocking only
function vetoProposal(uint proposalId) external {
    require(msg.sender == guardian, "GovernorBravo::veto: only guardian");
    require(state(proposalId) == ProposalState.Queued, "must be queued");
    proposals[proposalId].vetoed = true;
    // ✅ Emit event so community can observe and challenge guardian decisions
    emit ProposalVetoed(proposalId);
}

// Comptroller.sol — getAccountLiquidity()
function getAccountLiquidityInternal(address account)
    internal view returns (Error, uint, uint) {
    AccountLiquidityLocalVars memory vars;
    // ... (loops over all markets) ...
    // ❌ Oracle price fetched with no staleness check, no bounds check
    // ❌ A manipulated or stale oracle silently misprices collateral
    vars.oraclePrice = oracle.getUnderlyingPrice(asset);
    if (vars.oraclePrice == 0) {
        return (Error.PRICE_ERROR, 0, 0);
    }
    // ❌ No check for oracle returning a price outside reasonable range
    // ❌ (e.g. 1000x the previous block price)
    vars.tokensToDenom = mul_(mul_(vars.collateralFactor, vars.exchangeRate), vars.oraclePrice);
    vars.sumCollateral = mul_ScalarTruncateAddUInt(vars.tokensToDenom,
        vars.cTokenBalance, vars.sumCollateral);
}

// Comptroller.sol — getAccountLiquidity() with oracle safeguards
function getAccountLiquidityInternal(address account)
    internal view returns (Error, uint, uint) {
    AccountLiquidityLocalVars memory vars;
    // ... (loops over all markets) ...
    vars.oraclePrice = oracle.getUnderlyingPrice(asset);
    // ✅ Revert on zero price — prevents collateral being valued at 0
    if (vars.oraclePrice == 0) {
        return (Error.PRICE_ERROR, 0, 0);
    }
    // ✅ Staleness check — oracle must have updated within N blocks
    // ✅ (requires oracle to expose last-update timestamp)
    require(oracle.lastUpdated(asset) >= block.number - MAX_ORACLE_STALENESS,
        "oracle price stale");
    // ✅ Sanity bounds — revert if price deviates > 50% from TWAP reference
    uint twapPrice = twapOracle.getUnderlyingPrice(asset);
    require(
        vars.oraclePrice >= twapPrice / 2 && vars.oraclePrice <= twapPrice * 2,
        "oracle price out of bounds"
    );
    vars.tokensToDenom = mul_(mul_(vars.collateralFactor, vars.exchangeRate), vars.oraclePrice);
    vars.sumCollateral = mul_ScalarTruncateAddUInt(vars.tokensToDenom,
        vars.cTokenBalance, vars.sumCollateral);
}

// Comptroller.sol — _setPriceOracle()
function _setPriceOracle(PriceOracle newOracle) public returns (uint) {
    // ❌ Admin-only with no Timelock requirement in the contract itself
    // ❌ Any address holding adminship can replace oracle instantly
    if (msg.sender != admin) {
        return fail(Error.UNAUTHORIZED, FailureInfo.SET_PRICE_ORACLE_OWNER_CHECK);
    }
    PriceOracle oldOracle = oracle;
    oracle = newOracle;
    emit NewPriceOracle(oldOracle, newOracle);
    return uint(Error.NO_ERROR);
}

// Comptroller.sol — _setPriceOracle() with enforced timelock
// ✅ Require oracle changes to be submitted via timelock-governed pendingOracle
address public pendingOracle;
uint public pendingOracleTimestamp;
uint public constant ORACLE_UPDATE_DELAY = 2 days;

function _queuePriceOracle(PriceOracle newOracle) public {
    require(msg.sender == admin, "not admin");
    pendingOracle = address(newOracle);
    // ✅ Two-step: queue first, execute after delay — gives community time to react
    pendingOracleTimestamp = block.timestamp + ORACLE_UPDATE_DELAY;
    emit PriceOracleQueued(address(newOracle), pendingOracleTimestamp);
}

function _executePriceOracle() public {
    require(msg.sender == admin, "not admin");
    // ✅ Enforces delay in the contract itself — not reliant on Timelock config
    require(block.timestamp >= pendingOracleTimestamp, "delay not elapsed");
    require(pendingOracle != address(0), "no pending oracle");
    PriceOracle oldOracle = oracle;
    oracle = PriceOracle(pendingOracle);
    pendingOracle = address(0);
    emit NewPriceOracle(oldOracle, oracle);
}

// Comptroller.sol — claimComp() (simplified)
function claimComp(address holder) public {
    // ❌ No pause check — this function cannot be stopped by any
    // ❌ existing mechanism short of a full governance cycle
    return claimCompInternal(holder, allMarkets);
}

function claimCompInternal(address holder, CToken[] memory cTokens) internal {
    address[] memory holders = new address[](1);
    holders[0] = holder;
    // ❌ Processes all markets with no rate limit or claim cap
    claimComp(holders, cTokens, true, true);
}

// Comptroller.sol — claimComp() with guardian-controlled pause
// ✅ Independent pause flag for COMP claims — guardian can stop claims
// ✅ immediately without a governance vote
bool public compClaimsPaused = false;

function _setCompClaimsPaused(bool state) public returns (bool) {
    // ✅ Guardian (not only admin) can pause claims — faster response time
    require(msg.sender == guardian || msg.sender == admin, "not guardian or admin");
    compClaimsPaused = state;
    emit ActionPaused("Comp Claims", state);
    return state;
}

function claimComp(address holder) public {
    // ✅ Check pause flag before any distribution logic runs
    require(!compClaimsPaused, "Comptroller: comp claims are paused");
    return claimCompInternal(holder, allMarkets);
}

// CToken.sol — redeemFresh()
function redeemFresh(address payable redeemer, uint redeemTokensIn,
    uint redeemAmountIn) internal returns (uint) {
    // ❌ No nonReentrant modifier — relies entirely on CEI ordering
    // ❌ ERC-777 tokens with transfer hooks could re-enter here
    // ... validation and state updates ...
    totalSupply = totalSupply - vars.redeemTokens;
    accountTokens[redeemer] = accountTokens[redeemer] - vars.redeemTokens;
    // ← reentrancy surface for ERC-777 tokensReceived callback
    doTransferOut(redeemer, vars.redeemAmount);
    emit Redeem(redeemer, vars.redeemAmount, vars.redeemTokens);
    return uint(Error.NO_ERROR);
}

// CToken.sol — redeemFresh() with explicit reentrancy guard
uint private _reentrancyLock = 1; // ✅ storage-based lock (not memory — survives calls)

modifier nonReentrant() {
    // ✅ Explicit guard documents intent and prevents cross-function reentrancy
    require(_reentrancyLock == 1, "reentrant call");
    _reentrancyLock = 2;
    _;
    _reentrancyLock = 1;
}

function redeemFresh(address payable redeemer, uint redeemTokensIn,
    uint redeemAmountIn) internal nonReentrant returns (uint) {
    // ✅ Guard added — CEI pattern retained AND guard provides belt-and-suspenders
    totalSupply = totalSupply - vars.redeemTokens;
    accountTokens[redeemer] = accountTokens[redeemer] - vars.redeemTokens;
    doTransferOut(redeemer, vars.redeemAmount);
    emit Redeem(redeemer, vars.redeemAmount, vars.redeemTokens);
    return uint(Error.NO_ERROR);
}

// Comp.sol — delegate()
function delegate(address delegatee) public {
    // INFORMATIONAL: No restriction on who can be delegated to
    // No time-lock on delegation changes
    // No minimum holding period before delegated votes are counted
    return _delegate(msg.sender, delegatee);
}

// Comp.sol — delegate() with optional time-weighted approach (illustrative)
// ✅ This is one mitigation pattern — tradeoffs exist for each approach
mapping(address => uint) public delegationTimestamp;
uint public constant DELEGATION_DELAY = 1 days;

function delegate(address delegatee) public {
    // ✅ Track delegation timestamp — prevents flash delegation attacks
    delegationTimestamp[msg.sender] = block.timestamp;
    return _delegate(msg.sender, delegatee);
}

// ✅ Modified getPriorVotes to weight by delegation age
// ✅ Votes delegated within DELEGATION_DELAY count at partial weight
// Note: full implementation requires checkpoint integration

// distributeSupplierComp() — Current (Vulnerable)
function distributeSupplierComp(address cToken, address supplier) internal {
    CompMarketState storage supplyState = compSupplyState[cToken];
    Double memory supplyIndex = Double({mantissa: supplyState.index});
    Double memory supplierIndex = Double({mantissa: compSupplierIndex[cToken][supplier]});
    // ❌ Writing index before zero-check — initialization fires against wrong base
    compSupplierIndex[cToken][supplier] = supplyIndex.mantissa;
    if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {
        supplierIndex.mantissa = compInitialIndex;
    }
    Double memory deltaIndex = sub_(supplyIndex, supplierIndex);
    uint supplierTokens = CToken(cToken).balanceOf(supplier);
    uint supplierDelta = mul_(supplierTokens, deltaIndex);
    // ❌ No cap — can exceed reservoir balance
    uint supplierAccrued = add_(compAccrued[supplier], supplierDelta);
    compAccrued[supplier] = supplierAccrued;
}

// distributeSupplierComp() — Fixed
function distributeSupplierComp(address cToken, address supplier) internal {
    CompMarketState storage supplyState = compSupplyState[cToken];
    Double memory supplyIndex = Double({mantissa: supplyState.index});
    Double memory supplierIndex = Double({mantissa: compSupplierIndex[cToken][supplier]});
    // ✅ Zero-check fires BEFORE index is written — initialization is correct
    if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {
        supplierIndex.mantissa = compInitialIndex;
    }
    // ✅ Write happens after initialization — ordering is correct
    compSupplierIndex[cToken][supplier] = supplyIndex.mantissa;
    Double memory deltaIndex = sub_(supplyIndex, supplierIndex);
    uint supplierTokens = CToken(cToken).balanceOf(supplier);
    uint supplierDelta = mul_(supplierTokens, deltaIndex);
    uint supplierAccrued = add_(compAccrued[supplier], supplierDelta);
    // ✅ Cap to reservoir balance — prevents distributing more than exists
    uint reservoirBalance = EIP20Interface(getCompAddress()).balanceOf(address(this));
    compAccrued[supplier] = supplierAccrued > reservoirBalance ? reservoirBalance : supplierAccrued;
    emit DistributedSupplierComp(CToken(cToken), supplier, supplierDelta, supplyIndex.mantissa);
}

// GovernorBravo.sol — execute()
function execute(uint proposalId) external payable {
    require(state(proposalId) == ProposalState.Queued, "not queued");
    Proposal storage proposal = proposals[proposalId];
    // ❌ No emergency stop — queued proposals cannot be blocked
    proposal.executed = true;
    for (uint i = 0; i < proposal.targets.length; i++) {
        timelock.executeTransaction{value: proposal.values[i]}(
            proposal.targets[i], proposal.values[i],
            proposal.signatures[i], proposal.calldatas[i], proposal.eta
        );
    }
    emit ProposalExecuted(proposalId);
}

// GovernorBravo.sol — execute() + vetoProposal()
function execute(uint proposalId) external payable {
    require(state(proposalId) == ProposalState.Queued, "not queued");
    Proposal storage proposal = proposals[proposalId];
    // ✅ Veto check — guardian can block during timelock window without governance cycle
    require(!proposal.vetoed, "proposal vetoed by guardian");
    proposal.executed = true;
    for (uint i = 0; i < proposal.targets.length; i++) {
        timelock.executeTransaction{value: proposal.values[i]}(
            proposal.targets[i], proposal.values[i],
            proposal.signatures[i], proposal.calldatas[i], proposal.eta
        );
    }
    emit ProposalExecuted(proposalId);
}

// ✅ Guardian has narrow power: block execution only, cannot pass new proposals
function vetoProposal(uint proposalId) external {
    require(msg.sender == guardian, "not guardian");
    require(state(proposalId) == ProposalState.Queued, "must be queued");
    proposals[proposalId].vetoed = true;
    emit ProposalVetoed(proposalId); // ✅ Transparent — community can verify and challenge
}

// Comptroller.sol — getAccountLiquidityInternal()
vars.oraclePrice = oracle.getUnderlyingPrice(asset);
// ❌ No staleness check — accepts any non-zero price regardless of age
// ❌ No bounds check — 100x price move accepted silently
if (vars.oraclePrice == 0) {
    return (Error.PRICE_ERROR, 0, 0);
}

// Comptroller.sol — getAccountLiquidityInternal() with oracle safeguards
vars.oraclePrice = oracle.getUnderlyingPrice(asset);
if (vars.oraclePrice == 0) {
    return (Error.PRICE_ERROR, 0, 0);
}
// ✅ Require oracle freshness — enforces data recency guarantee
(,, uint updatedAt,) = AggregatorV3Interface(oracleFeed[asset]).latestRoundData();
require(block.timestamp - updatedAt <= MAX_ORACLE_DELAY, "stale oracle");
// ✅ Sanity bound — revert if price deviates >50% from TWAP reference
uint twap = twapOracle.getUnderlyingPrice(asset);
require(vars.oraclePrice >= twap / 2 && vars.oraclePrice <= twap * 2, "price outlier");