Code4rena · S-175 · Apr 2025

Virtuals Protocol

HIGH✓ ACCEPTED

// Manual ContributionNFT Minting to Arbitrary Addresses Creates Governance and Functional Risks

01 /

EXECUTIVE SUMMARY

EXECUTIVE_SUMMARY.log

ISSUE OVERVIEW›The protocol inconsistently handles asset ownership between ServiceNFT (auto-minted to TBA) and ContributionNFT (manually minted to any address). Proposers can direct NFTs to non-TBA wallets, breaking the TBA-centric ownership model and blocking voters from accessing isModel metadata before casting votes.

AFFECTED COMPONENT›ContributionNft.sol — mint() function (L49–51, L66, L82)

RISK CLASSIFICATION›Governance Integrity + Functional Risk

SEVERITY›HIGH

STATUS›ACCEPTED

02 /

ROOT CAUSE ANALYSIS

ROOT_CAUSE_ANALYSIS.log

The ContributionNFT mint() function accepts a caller-controlled `to` parameter with no validation, allowing proposers to mint to any address rather than the TBA enforced by ServiceNFT. Additionally, the `isModel` flag is only resolvable after the NFT is minted, leaving voters without proposal type context during the voting window.

ContributionNft.sol — L49-51 (vulnerable) vs ServiceNFT (correct)

// @audit ContributionNFT — caller controls destination
function mint(address to, uint256 proposalId, ...) external {
    _mint(to, proposalId); // 'to' is unvalidated
}

// ServiceNFT — correctly enforces TBA model
function mint(uint256 proposalId, ...) external {
    _mint(info.tba, proposalId); // always mints to TBA ✓
}

// propose() — missing isModel context for voters
function propose(
    address[] memory targets,
    uint256[] memory values,
    bytes[] memory calldatas,
    string memory description
) public override(GovernorUpgradeable) returns (uint256) {
    // @audit isModel is NOT included here
    // voters cannot distinguish Model vs Dataset proposals
}

▸ INCORRECT ASSUMPTIONS

—Proposers will always mint ContributionNFT to the correct TBA address without enforcement
—Governance voters have access to isModel metadata before the voting window opens
—The TBA-centric ownership model is enforced uniformly across all NFT types

▸ AFFECTED CONTRACTS

ContributionNft.sol (L49–51, L66, L82)
GovernorUpgradeable.sol — propose()

03 /

ATTACK SCENARIO

ATTACK_VECTOR — Governance Manipulation via Off-TBA NFT Minting

Malicious proposer calls propose() to create a governance proposal targeting a ServiceNFT or protocol parameter.

↓

Proposer calls ContributionNFT.mint() with an arbitrary wallet address (not the correct TBA), directing the NFT outside the TBA-centric model.

↓

Voters enter the voting window but cannot resolve isModel — they cannot determine whether the proposal targets a Model or a Dataset.

↓

Proposer holds the ContributionNFT outside TBA, enabling governance manipulation via off-protocol NFT control and breaking the ownership invariant the entire system assumes.

04 /

IMPACT ASSESSMENT

HIGH

Governance Integrity

ContributionNFTs held outside TBAs break the protocol's core ownership invariant. Proposers gain illegitimate control over NFTs that should be TBA-bound, enabling off-protocol governance leverage.

MEDIUM

Information Asymmetry

Voters cannot distinguish Model vs Dataset proposals before casting votes. The isModel field is only resolvable after ContributionNFT is minted — a step fully controlled by the proposer.

HIGH

Protocol Integrity

The inconsistency between ServiceNFT's enforced TBA minting and ContributionNFT's arbitrary minting undermines the uniform ownership model the protocol's security relies on.

05 /

MITIGATION

Remove the arbitrary `to` parameter from ContributionNFT.mint() and enforce TBA-bound minting at the contract level. Add isModel context to propose() so voters have complete proposal information.

— VULNERABLEimplementation

function mint(address to, uint256 proposalId) external {
    _mint(to, proposalId); // caller controls destination
}

+ SECUREimplementation

function mint(uint256 proposalId) external {
    address tba = IAgentNft(personaNft).virtualInfo(virtualId).tba;
    _mint(tba, proposalId); // always mints to TBA
}

// Also add isModel to propose() calldata or description
// so voters can resolve proposal type without waiting for mint

▸ SECURITY CONSIDERATIONS

01.Audit all other NFT minting paths in the protocol for the same TBA enforcement gap
02.Consider emitting a VotingContextSet event after ContributionNFT mint to signal voter readiness
03.Add integration tests that verify ContributionNFT always lands in TBA after propose()

06 /

RESEARCHER NOTES

RESEARCHER_NOTES.md

@preetsinghmakkar— personal analysis

I discovered this by comparing minting patterns across all NFT contracts in the Virtuals codebase. ServiceNFT hardcodes the TBA as recipient — ContributionNFT exposes a `to` parameter with zero validation. The asymmetry was the first red flag.

Protocols that enforce ownership invariants in one place but not another almost always have consistency bugs. Once I saw `_mint(info.tba, proposalId)` in ServiceNFT and `_mint(to, proposalId)` in ContributionNFT, I knew the assumption wasn't being enforced where it mattered.

The isModel gap was a secondary discovery. While tracing the field through the codebase I realized it only existed in ContributionNFT — which the proposer controls and can delay minting indefinitely. Worst case: proposer deliberately withholds the mint to obscure proposal intent during voting.

This finding reinforced a rule I now apply to every NFT-heavy codebase: find every ownership invariant stated in docs or comments, then verify it's enforced uniformly across all minting paths. Trust-but-verify at the contract level, not just the protocol level.

07 /