[Security] WBERAStakerVault: ERC4626 inflation/donation attack — no _decimalsOffset override

[sailorpepe]

Summary

WBERAStakerVault inherits ERC4626Upgradeable without overriding _decimalsOffset() (defaults to 0). Combined with the permissionless receiveRewards(uint256) function, this enables the classic ERC4626 share price inflation attack against first depositors.

Root Cause

• No virtual shares: _decimalsOffset() is not overridden, so no virtual shares protection exists.
• Permissionless donation: receiveRewards(uint256) has no access control — anyone can donate WBERA to inflate totalAssets().

Attack Path

1. Attacker is the first depositor — deposits 1 wei WBERA → receives 1 share
2. Attacker calls receiveRewards(1e18) → donates 1e18 WBERA directly to vault
3. Share price = totalAssets / totalShares = (1e18 + 1) / 1 ≈ 1e18 per share
4. Victim deposits 1.99e18 WBERA → previewDeposit = 1.99e18 * 1 / (1e18+1) = 1 share (truncated)
5. Attacker now owns 50% of vault (1 of 2 shares) but funded ~1e18 of ~3e18 total
6. Attacker withdraws → receives ~1.5e18, profiting ~0.5e18 at victim's expense

Existing Mitigation

The NatSpec mentions "Inflation Attack Protection: Initial deposit mechanism" — this is an operational mitigation (governance deposits first), but it is not enforced in code. If governance forgets or the vault is redeployed without the initial deposit, the attack surface exists.

Recommended Fix

Override _decimalsOffset() to add code-level virtual shares protection:

function _decimalsOffset() internal pure override returns (uint8) {
    return 3; // creates 1000 virtual shares per asset unit
}

Alternatively or additionally, add access control to receiveRewards:

function receiveRewards(uint256 amount) external onlyRole(MANAGER_ROLE) {
    WBERA.safeTransferFrom(msg.sender, address(this), amount);
    emit RewardsReceived(msg.sender, amount, totalAssets());
}

References

• OpenZeppelin ERC4626 Security Considerations
• WBERAStakerVault.sol#L269
• WBERAStakerVault.sol#L145

Severity

Medium — Exploitable only before initial governance deposit. Operational mitigation exists but is not code-enforced.

[sailorpepe]

⚠️ Correction — Severity Downgrade

After reviewing the actual OpenZeppelin v5.1.0 ERC4626Upgradeable implementation used in this project, I need to correct my analysis.

What I got wrong

The _convertToShares formula at line 248-249 includes +1 in both numerator and denominator:

shares = assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding)

With _decimalsOffset() = 0, this becomes: shares = assets * (totalSupply + 1) / (totalAssets + 1)

My original attack math was incorrect. I used assets * totalSupply / totalAssets (without the +1). The corrected math:

1. Attacker deposits 1 wei → 1 share
2. Attacker donates 1e18 via receiveRewards
3. Victim deposits 1.99e18 → shares = 1.99e18 * 2 / (1e18 + 2) ≈ 3 shares (not 1)
4. Attacker's 1/4 share ≈ 0.7475e18. Attacker invested 1e18+1. Net loss for attacker.

OpenZeppelin's own docs confirm: "the default offset (0) makes it non-profitable even if an attacker is able to capture value from multiple user deposits."

What remains valid

1. receiveRewards has no access control — anyone can donate WBERA to the vault. While not exploitable for the inflation attack due to the +1 protection, permissionless donations could still cause minor share price rounding issues for small depositors. Adding a MANAGER_ROLE check would be a defense-in-depth improvement.
2. Adding _decimalsOffset() would still be best practice for deeper defense, but is not strictly required given the +1 mitigation.

Revised Severity

Low / Informational (downgraded from Medium). The core inflation attack is not profitable. The permissionless receiveRewards is a hardening concern, not an active vulnerability.

Apologies for the incorrect initial analysis.