Summary
In the QuantAMMWeightedPoolFactory contract, there's functions to create and deploy Pools with and without params. Both functions are using a salt to create a unique pool. However, the salt used to create pool is bare and plain that anyone can use to create very same pool.
Vulnerability Details
QuantAMMWeightedPoolFactory::createWithoutArgs:
function createWithoutArgs(NewPoolParams memory params) external returns (address pool) {
if (params.roleAccounts.poolCreator != address(0)) {
revert StandardPoolWithCreator();
}
LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement();
liquidityManagement.enableDonation = params.enableDonation;
liquidityManagement.disableUnbalancedLiquidity = params.disableUnbalancedLiquidity;
pool = _create(
abi.encode(
QuantAMMWeightedPool.NewPoolParams({
name: params.name,
symbol: params.symbol,
numTokens: params.normalizedWeights.length,
version: "version",
updateWeightRunner: _updateWeightRunner,
poolRegistry: params.poolRegistry,
poolDetails: params.poolDetails
}),
getVault()
),
@> params.salt
);
QuantAMMWeightedPool(pool).initialize(
params._initialWeights,
params._poolSettings,
params._initialMovingAverages,
params._initialIntermediateValues,
params._oracleStalenessThreshold
);
_registerPoolWithVault(
pool,
params.tokens,
params.swapFeePercentage,
false,
params.roleAccounts,
params.poolHooksContract,
liquidityManagement
);
}
QuantAMMWeightedPoolFactory::create:
function create(NewPoolParams memory params) external returns (address pool, bytes memory poolArgs) {
if (params.roleAccounts.poolCreator != address(0)) {
revert StandardPoolWithCreator();
}
LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement();
liquidityManagement.enableDonation = params.enableDonation;
liquidityManagement.disableUnbalancedLiquidity = params.disableUnbalancedLiquidity;
poolArgs = abi.encode(
QuantAMMWeightedPool.NewPoolParams({
name: params.name,
symbol: params.symbol,
numTokens: params.normalizedWeights.length,
version: "version",
updateWeightRunner: _updateWeightRunner,
poolRegistry: params.poolRegistry,
poolDetails: params.poolDetails
}),
getVault()
);
@> pool = _create(poolArgs, params.salt);
QuantAMMWeightedPool(pool).initialize(
params._initialWeights,
params._poolSettings,
params._initialMovingAverages,
params._initialIntermediateValues,
params._oracleStalenessThreshold
);
_registerPoolWithVault(
pool,
params.tokens,
params.swapFeePercentage,
false,
params.roleAccounts,
params.poolHooksContract,
liquidityManagement
);
}
It's a bare salt anyone can copy the salt and deploy the same pool. There should be a re-calculation of the hash with the addition of block.timestamp because the inherited create function does not hash the salt with block.timestamp. This creates a vulnerability related to front-running by a malicious msg.sender.
The issue arises because the msg.sender in the parent contract is always the same (QuantAMMWeightedPoolFactory), and the block.chainid remains unchanged. As a result, any malicious msg.sender can deploy the same pool using the same salt, block.chainid, and msg.sender. Adding block.timestamp to the salt ensures that the salt is unique and secure.
@balancer-labs/v3-pool-utils/contracts/BasePoolFactory.sol::BasePoolFactory::_create:
function _computeFinalSalt(bytes32 salt) internal view virtual returns (bytes32) {
@> return keccak256(abi.encode(msg.sender, block.chainid, salt));
}
function _create(bytes memory constructorArgs, bytes32 salt) internal returns (address pool) {
bytes memory creationCode = abi.encodePacked(_creationCode, constructorArgs);
@> bytes32 finalSalt = _computeFinalSalt(salt);
pool = Create2.deploy(0, finalSalt, creationCode);
_registerPoolWithFactory(pool);
}
Above is the parent _create function which calling _computeFinalSalt function to compute a final unique salt. Due to the pool factory, the msg.sender will always be the pool factory and in the mempool we will have salt that we can copy and precompute and replay.
Impact
-
Replay Attacks on different chains
Without a unique salt, attackers could attempt to replay a pool creation transaction on different chains or environments.
-
Precomputation of pool addresses
If an attacker can predict the pool address (because the salt is plain and deterministic), they might:
Front-run the pool creation by deploying a malicious contract to the same address.
Exploit the predictable address to manipulate on-chain interactions with the pool (e.g., feeding fake oracle data).
Tools Used
Manual review
Recommendations
We can use block.number and/or block.timestamp to enhance the final salt with an additional layer of security, ensuring its uniqueness.
We can do something like below:
QuantAMMWeightedPoolFactory::createWithoutArgs:
function createWithoutArgs(NewPoolParams memory params) external returns (address pool) {
if (params.roleAccounts.poolCreator != address(0)) {
revert StandardPoolWithCreator();
}
LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement();
liquidityManagement.enableDonation = params.enableDonation;
// disableUnbalancedLiquidity must be set to true if a hook has the flag enableHookAdjustedAmounts = true.
liquidityManagement.disableUnbalancedLiquidity = params.disableUnbalancedLiquidity;
+ bytes32 semiFinalSalt = keccak256(abi.encode(block.number, block.timestamp, params.salt));
pool = _create(
abi.encode(
QuantAMMWeightedPool.NewPoolParams({
name: params.name,
symbol: params.symbol,
numTokens: params.normalizedWeights.length,
version: "version",
updateWeightRunner: _updateWeightRunner,
poolRegistry: params.poolRegistry,
poolDetails: params.poolDetails
}),
getVault()
),
+ semiFinalSalt
- params.salt
);
QuantAMMWeightedPool(pool).initialize(
params._initialWeights,
params._poolSettings,
params._initialMovingAverages,
params._initialIntermediateValues,
params._oracleStalenessThreshold
);
_registerPoolWithVault(
pool,
params.tokens,
params.swapFeePercentage,
false, // not exempt from protocol fees
params.roleAccounts,
params.poolHooksContract,
liquidityManagement
);
}
QuantAMMWeightedPoolFactory::create:
function create(NewPoolParams memory params) external returns (address pool, bytes memory poolArgs) {
if (params.roleAccounts.poolCreator != address(0)) {
revert StandardPoolWithCreator();
}
LiquidityManagement memory liquidityManagement = getDefaultLiquidityManagement();
liquidityManagement.enableDonation = params.enableDonation;
// disableUnbalancedLiquidity must be set to true if a hook has the flag enableHookAdjustedAmounts = true.
liquidityManagement.disableUnbalancedLiquidity = params.disableUnbalancedLiquidity;
poolArgs = abi.encode(
QuantAMMWeightedPool.NewPoolParams({
name: params.name,
symbol: params.symbol,
numTokens: params.normalizedWeights.length,
version: "version",
updateWeightRunner: _updateWeightRunner,
poolRegistry: params.poolRegistry,
poolDetails: params.poolDetails
}),
getVault()
);
+ bytes32 semiFinalSalt = keccak256(abi.encode(block.number, block.timestamp, params.salt));
- pool = _create(poolArgs, params.salt);
+ pool = _create(poolArgs, semiFinalSalt);
QuantAMMWeightedPool(pool).initialize(
params._initialWeights,
params._poolSettings,
params._initialMovingAverages,
params._initialIntermediateValues,
params._oracleStalenessThreshold
);
_registerPoolWithVault(
pool,
params.tokens,
params.swapFeePercentage,
false, // not exempt from protocol fees
params.roleAccounts,
params.poolHooksContract,
liquidityManagement
);
}
