Auditoría Trail of Bits — CultivaYield Protocol

Resumen Ejecutivo

CultivaYield Protocol presenta 3 vulnerabilidades CRITICAL que deben resolverse antes del lanzamiento en mainnet. La más grave es una vulnerabilidad de reentrancia en YieldVault.withdraw() que permitiría drenar los fondos del protocolo. El oráculo TWAP custom carece de validación de freshness y circuit breaker, exponiéndolo a manipulación de precio en bloques vacíos de Arbitrum. El patrón UUPS no tiene timelock, permitiendo upgrades maliciosos instantáneos. La cobertura de tests es del 42%, insuficiente para un protocolo con TVL esperado de $500k-2M. Recomendación: no desplegar en mainnet hasta resolver todos los items CRITICAL y HIGH.

Documentación & Especificaciones

Plain English, NatSpec, diagramas arquitecturales

Descripción del Sistema (Generada)

CultivaYield permite depositar LP tokens de Uniswap V3 a cambio de shares que acumulan recompensas $CULT. El vault usa patrón ERC-4626 adaptado con contabilidad de shares. Las recompensas se minan on-demand via RewardDistributor cuando el usuario retira o hace claim.

Supuestos Clave Identificados

✓LP tokens siguen ERC-20 estándar
●TWAP confiable en bloques normales
✗Sin doc de invariantes del protocolo
✗Sin especificación de comportamiento ante pausas

Medium CY-DOC-001 NatSpec ausente en funciones críticas YieldVault.sol, RewardDistributor.sol

Las funciones withdraw(), deposit() y claimFor() carecen de comentarios NatSpec (@notice, @param, @return, @dev). Esto dificulta la auditoría, genera ambigüedad sobre precondiciones y aumenta el riesgo de uso incorrecto por integradores externos.

✗YieldVault.sol — 0 de 8 funciones públicas tienen NatSpec completo
✗RewardDistributor.sol — Sin documentar condiciones de error
●CultToken.sol — Parcial (solo @title)
✓Interfaces — Descripciones básicas presentes

Acción Recomendada

Añadir NatSpec completo a todas las funciones públicas/external antes de auditoría externa. Esfuerzo: 1-2 días.

Arquitectura On-Chain / Off-Chain

Distribución de componentes, trust boundaries, flujo de datos

Flujo de Contratos — CultivaYield Protocol

Usuario

EOA / Wallet

UUPSProxy.sol

Admin: Multisig 2/3

YieldVault.sol

Implementación

RewardDistributor

⚠ Call externo

YieldVault.sol

totalAssets()

CultivaOracle.sol

⚠ TWAP sin validar

UniswapV3Pool

Fuente de precios

Medium CY-ARCH-001 Trust boundary no documentado entre Vault y Oracle YieldVault.sol, CultivaOracle.sol

El vault confía implícitamente en el precio devuelto por CultivaOracle.sol para calcular totalAssets() y determinar la cantidad de LP tokens a entregar en retiros. No hay validación del precio ni mecanismo de pausa si el oráculo devuelve un valor aberrante.

Recomendación

Documentar explícitamente los trust assumptions del oráculo. Añadir circuit breaker que pause el vault si el precio varía más del 20% entre bloques consecutivos.

Proxy UUPS & Upgradeabilidad

Patrón de upgrade, storage layout, inicialización

Critical CY-UPG-001 Upgrade UUPS sin timelock — admin puede actualizar instantáneamente UUPSProxy.sol, YieldVault.sol

El contrato YieldVault usa patrón UUPS (EIP-1822) donde la función de upgrade está protegida por el multisig 2-of-3. Sin embargo, no hay timelock entre la aprobación del upgrade y su ejecución. Cualquier compromiso del multisig (o colusión de 2 firmantes) permite reemplazar la implementación instantáneamente, drenando todo el TVL.

              Solidity
              ⚠ VULNERABLE
            
// YieldVault.sol — función upgradeable sin timelock
function _authorizeUpgrade(address newImplementation)
    internal
    override
    onlyOwner  // Multisig puede ejecutar upgrade INMEDIATAMENTE
{}

// No hay TimelockController ni delay

              Solidity
              ✓ CORRECCIÓN
            
// Solución: integrar OpenZeppelin TimelockController
import "@openzeppelin/contracts/governance/TimelockController.sol";

// Configurar con delay mínimo de 48 horas
TimelockController public timelock;
uint256 public constant UPGRADE_DELAY = 2 days;

function _authorizeUpgrade(address newImplementation)
    internal
    override
    onlyRole(UPGRADER_ROLE)  // Solo desde timelock
{
    require(msg.sender == address(timelock), "Must go through timelock");
}

Impacto

Con $500k-2M TVL esperado, un upgrade malicioso instantáneo puede drenar todo el protocolo en una sola transacción. Clasificación: CRITICAL — no desplegar sin timelock.

High CY-UPG-002 Storage layout no verificado en upgrades futuros YieldVault.sol

No hay documentación del storage layout actual ni tests que verifiquen que futuras implementaciones no colisionan con el slot existente. Una colisión de storage en un upgrade puede corromper los balances de todos los usuarios.

✗Sin archivo STORAGE.md con layout documentado
✗Sin test de validación de storage slots
✓Hereda de OZ UUPSUpgradeable (slots reservados con __gap)

Acción

Usar slither --print variable-order para documentar el layout. Añadir test con hardhat-storage-layout al pipeline CI. Esfuerzo: 0.5 días.

Revisión de Implementación

Funciones, herencia, eventos, pitfalls comunes

Critical CY-IMPL-001 Reentrancia en withdraw() — Llamada externa antes de actualizar estado YieldVault.sol (sin número de línea provisto)

La función withdraw() transfiere LP tokens al usuario y luego llama al contrato externo RewardDistributor.claimFor(). Aunque tiene el modificador nonReentrant, la quema de shares se realiza ANTES de la transferencia de LP, pero el estado de recompensas (pendingRewards) no se resetea antes de la llamada a claimFor(). Un atacante con un contrato LP malicioso puede reentrar via el callback de transferencia.

              Solidity — YieldVault.sol:withdraw()
              ⚠ PATRÓN PELIGROSO
            
function withdraw(uint256 shares) external nonReentrant {
    uint256 amount = (shares * totalAssets()) / totalSupply();
    _burn(msg.sender, shares);         // ✓ Shares quemadas

    uint256 rewards = pendingRewards(msg.sender);  // Lee estado

    IERC20(lpToken).transfer(msg.sender, amount);  // ⚠ Llamada externa
    // Si lpToken tiene callback (ERC-777/hooks), puede reentrar aquí
    // pendingRewards[msg.sender] TODAVÍA no se ha reseteado

    IRewardDistributor(rewardDistributor).claimFor(msg.sender, rewards); // Mint $CULT
    emit Withdrawn(msg.sender, amount, rewards);
}

              Solidity
              ✓ PATRÓN CORRECTO (Checks-Effects-Interactions)
            
function withdraw(uint256 shares) external nonReentrant {
    // 1. CHECKS
    require(shares > 0, "Zero shares");
    require(balanceOf(msg.sender) >= shares, "Insufficient shares");

    // 2. EFFECTS — actualizar TODO el estado primero
    uint256 amount = (shares * totalAssets()) / totalSupply();
    uint256 rewards = pendingRewards(msg.sender);
    _resetPendingRewards(msg.sender);  // ← Resetear ANTES de llamadas externas
    _burn(msg.sender, shares);

    // 3. INTERACTIONS — llamadas externas al final
    IERC20(lpToken).safeTransfer(msg.sender, amount);
    IRewardDistributor(rewardDistributor).claimFor(msg.sender, rewards);

    emit Withdrawn(msg.sender, amount, rewards);
}

Critical CY-IMPL-002 Oráculo TWAP sin validación de freshness — manipulable en Arbitrum CultivaOracle.sol:getPrice()

El oráculo usa TWAP de 10 minutos (600 segundos) de Uniswap V3. En Arbitrum L2, los bloques pueden quedar vacíos durante periodos de inactividad del sequencer. Un atacante puede esperar un periodo de baja actividad para manipular el TWAP con operaciones grandes, afectando el cálculo de totalAssets() y permitiendo retirar más LP tokens de los que corresponden.

              Solidity — CultivaOracle.sol
              ⚠ SIN VALIDACIONES
            
function getPrice() external view returns (uint256) {
    uint32[] memory secondsAgos = new uint32[](2);
    secondsAgos[0] = 600;  // 10 minutos
    secondsAgos[1] = 0;

    (int56[] memory tickCumulatives,) =
        IUniswapV3Pool(pool).observe(secondsAgos);
    int56 tickDiff = tickCumulatives[1] - tickCumulatives[0];
    int24 avgTick = int24(tickDiff / 600);

    // ⚠ Sin validación de freshness
    // ⚠ Sin circuit breaker (precio puede ser 0 o extremo)
    // ⚠ Sin comparación con precio spot para detectar manipulación
    return TickMath.getSqrtRatioAtTick(avgTick);
}

✗Sin validación de que el pool tiene suficiente liquidez (cardinality check)
✗Sin circuit breaker para movimientos de precio >20% en una ventana
✗Sin verificación de sequencer uptime (Arbitrum L2)
✗Ventana de 10 min demasiado corta para L2 con bloques vacíos

Corrección

1. Usar Chainlink Sequencer Uptime Feed para Arbitrum antes de consultar precios.
2. Ampliar ventana TWAP a 30 minutos mínimo.
3. Añadir require(cardinality >= 30, "Insufficient observations").
4. Implementar circuit breaker: si precio varía >15%, pausar vault automáticamente.

High CY-IMPL-003 claimFor() sin límite de mint — inflación ilimitada de $CULT RewardDistributor.sol:claimFor()

La función claimFor(address user, uint256 amount) acepta cualquier amount sin validación de máximo. Si el vault tiene un bug en el cálculo de pendingRewards() (ej. overflow en períodos largos), o si el vault es comprometido, se puede mintear una cantidad arbitraria de $CULT, colapsando el tokenomics del protocolo.

              Solidity — RewardDistributor.sol
              ⚠ SIN CAPS
            
function claimFor(address user, uint256 amount) external {
    require(msg.sender == vault, "Only vault");
    // Sin validación de amount máximo
    // Sin cap de emisión diaria/semanal
    ICultToken(cultToken).mint(user, amount);  // Mint sin límite
}

Corrección

Añadir: require(amount <= MAX_CLAIM_PER_TX, "Exceeds max claim") y un cap de emisión diaria. Integrar con el schedule de emisión definido en el whitepaper. Validar que amount no excede las recompensas acumuladas del usuario.

High CY-IMPL-004 transfer() en lugar de safeTransfer() para LP tokens YieldVault.sol:withdraw()

El código usa IERC20(lpToken).transfer() que no verifica el valor de retorno (algunos tokens no siguen el estándar ERC-20 y devuelven false en lugar de revertir). Si la transferencia falla silenciosamente, el usuario pierde sus shares sin recuperar los LP tokens.

Corrección Inmediata

Reemplazar por IERC20(lpToken).safeTransfer(msg.sender, amount) usando SafeERC20 de OpenZeppelin. Cambio de 1 línea + 1 import. Esfuerzo: 30 minutos.

Herencia & Eventos

Contrato	Herencia	Estado	Observación
YieldVault.sol	UUPSUpgradeable, ReentrancyGuard, Pausable, Ownable2Step	OK	Herencia limpia, sin diamante
CultToken.sol	ERC20, Ownable, AccessControl	WARN	Ownable y AccessControl simultáneos, redundante
RewardDistributor.sol	Ownable	OK	Simple, sin riesgos
CultivaOracle.sol	— (standalone)	FAIL	Sin pausable, sin access control para emergencias

Cobertura de Eventos

✓Withdrawn(address, uint256, uint256) — emitido en retiro
✓Deposited(address, uint256, uint256) — emitido en depósito
✗Sin evento para cambios de oráculo (si admin actualiza dirección)
✗Sin evento PriceDeviation para alertas de manipulación
✗claimFor() no emite evento (imposible auditar flujo de rewards off-chain)
●Pausa/Despausa presentes pero sin indexed parameters

Dependencias & Librerías

Calidad, versiones, código copiado

Dependencia	Versión	Estado	Evaluación
@openzeppelin/contracts-upgradeable	4.9.3	DESACTUALIZADO	5.x disponible con mejoras de seguridad; migración recomendada
@uniswap/v3-core	1.0.0	OK	Versión estable y auditada
TickMath (copiado)	custom	RIESGO	Código copiado de Uniswap V3 sin referencia al commit original; 87 líneas
hardhat	2.19.0	OK	Actualizado

Medium CY-DEP-001 TickMath.sol copiado sin origen documentado contracts/ (TickMath implícito en Oracle)

El código de TickMath ha sido copiado y potencialmente modificado. No hay referencia al commit de Uniswap del que proviene ni tests que verifiquen que la implementación es idéntica. Diferencias en los cálculos de precisión podrían generar errores de precio acumulados.

Recomendación

Importar directamente desde @uniswap/v3-core/contracts/libraries/TickMath.sol via npm. Eliminar la copia local. Si se necesitan modificaciones, documentarlas explícitamente.

Suite de Tests & Verificación

Cobertura, fuzzing, integración, CI/CD

Tests Actuales

Cobertura total 42%

Hardhat (18 tests) 30% cov.

Foundry (12 tests) 12% cov.

Técnicas de Testing

✓Unit tests básicos (Hardhat)
✓Tests de integración parciales (Foundry)
✗Fuzzing / property-based (Echidna)
✗Invariant tests (Foundry)
✗Verificación formal
✗CI/CD pipeline (GitHub Actions)
✗Tests de reentrancia explícitos

High CY-TEST-001 Cobertura 42% — insuficiente para TVL de $500k-2M tests/, foundry/

Para un protocolo DeFi con TVL esperado de $500k-2M, la cobertura mínima aceptable antes de auditoría externa es 90%+. Los casos edge más críticos no están cubiertos:

✗Retiro cuando vault está pausado
✗Depósito/retiro con totalSupply() == 0 (primer depósito)
✗Comportamiento con precio del oráculo = 0 o MAX_UINT
✗Ataque de reentrancia (test adversarial)
✗Upgrade y migración de datos

Invariantes Recomendadas (Foundry)

1. sum(shares) == totalSupply() siempre
2. totalAssets() > 0 cuando hay usuarios activos
3. Un usuario no puede retirar más LP de lo que depositó (ajustado por recompensas)
4. $CULT emitido <= schedule de emisión en whitepaper

Plan de Testing

Semana 1: Aumentar cobertura a 80% con Hardhat. Semana 2: Añadir invariant tests en Foundry + Echidna fuzzing 4 horas. Semana 3: CI/CD con cobertura mínima del 90% como gate. Antes de mainnet.

▼

Roadmap de Mejoras Priorizado

Plan de acción antes del despliegue en mainnet (3 semanas)

CRITICAL — Resolver antes de cualquier despliegue

Implementar TimelockController para upgrades UUPS (CY-UPG-001)

Desplegar TimelockController con delay 48h. Transferir UPGRADER_ROLE al timelock. Sin esto, el protocolo puede ser drenado en 1 tx.

Esfuerzo: 2 días

Archivo: UUPSProxy.sol, YieldVault.sol

Riesgo si se omite: Pérdida total TVL

Refactorizar withdraw() a patrón Checks-Effects-Interactions (CY-IMPL-001)

Mover _resetPendingRewards() antes de cualquier llamada externa. Añadir test de ataque de reentrancia.

Esfuerzo: 1 día

Archivo: YieldVault.sol

Riesgo si se omite: Drenado de rewards

Añadir validaciones al oráculo TWAP + Chainlink Sequencer Feed (CY-IMPL-002)

Integrar AggregatorV3Interface de Chainlink para verificar sequencer uptime en Arbitrum. Ampliar ventana a 30 min. Añadir circuit breaker de ±15%.

Esfuerzo: 3 días

Archivo: CultivaOracle.sol

Riesgo si se omite: Manipulación precio

HIGH — Resolver antes de mainnet

Añadir cap de emisión a claimFor() (CY-IMPL-003)

Esfuerzo: 0.5 días

Archivo: RewardDistributor.sol

Migrar transfer() a safeTransfer() con SafeERC20 (CY-IMPL-004)

Esfuerzo: 0.5 horas

Archivo: YieldVault.sol

Aumentar cobertura de tests al 90%+ con invariant tests (CY-TEST-001)

Esfuerzo: 1-2 semanas

Herramientas: Foundry, Echidna

Documentar storage layout y añadir test de migración (CY-UPG-002)

Esfuerzo: 0.5 días

Herramienta: slither --print variable-order

MEDIUM — Para calidad de producción

NatSpec completo en todas las funciones públicas (CY-DOC-001)

Esfuerzo: 1-2 días

Eliminar código TickMath copiado, usar npm (CY-DEP-001)

Esfuerzo: 2 horas

Añadir eventos en claimFor() y cambios de oráculo (CY-IMPL-005)

Esfuerzo: 0.5 días

LOW — Nice to have

11+

Unificar Ownable y AccessControl en CultToken.sol · Actualizar OZ 4.9 a 5.x · CI/CD con cobertura gate · Diagrama arquitectural Slither

Esfuerzo combinado: 3-4 días

Post-lanzamiento V2

Evaluación General: MADUREZ BAJA — No apto para mainnet

El codebase muestra conocimiento de los patrones DeFi pero carece de las medidas de seguridad críticas para gestionar TVL real. Los 3 items CRITICAL deben resolverse completamente antes de cualquier despliegue. Tras corregirlos, se recomienda una auditoría externa (Trail of Bits, Certik o Consensys Diligence) de 2-3 semanas con el codebase final.

Timeline estimado hacia mainnet: Semana 1: CRITICAL items · Semana 2-3: HIGH items + tests · Semana 4-6: Auditoría externa · Semana 7: Resolución de hallazgos · Semana 8: Mainnet con documentación de limitaciones conocidas.

Trail of Bits Guidelines Solidity 0.8.20 Arbitrum L2 UUPS Proxy DeFi Yield Farming 5 fases CEI Pattern TWAP Oracle