Summary
There is a logical discrepancy in how Market::wethRewardChangeX18 is computed in Market::getVaultAccumulatedValues. Unlike the other reward values (realized debt, unrealized debt, and USDC credit), Market::wethRewardChangeX18 is not multiplied by the vault’s share of total delegated credit (vaultCreditShareX18). Additionally, the code does not check whether the vault has previously distributed rewards (i.e., if lastVaultDistributedWethRewardPerShareX18 is zero) before calculating changes.
Vulnerability Details
Incorrect Reward Calculation:
The function correctly multiplies other reward deltas (realizedDebtChangeUsdX18, unrealizedDebtChangeUsdX18, usdcCreditChangeX18) by the vault’s credit share (vaultCreditShareX18) as the function is called getVaultAccumulatedValues and the natspec is as follows:
However, wethRewardChangeX18 is calculated only as:
wethRewardChangeX18 = ud60x18(self.wethRewardPerVaultShare)
.sub(lastVaultDistributedWethRewardPerShareX18);
without factoring in vaultCreditShareX18. This discrepancy calculates the wethRewardChange for the market instead of the reward the vault is entitled to.
No Check for Zero Distribution:
The other reward calculations do something like:
realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
? ...
: SD59x18_ZERO;
This condition bypasses calculations if the vault has not previously distributed any rewards. By contrast, wethRewardChangeX18 has no similar check, which may lead to incorrect initial distribution logic.
This leads to further discrepancies as the following lines are in Vault::_recalculateConnectedMarketsState:
if (!market.getTotalDelegatedCreditUsd().isZero()) {
(
ctx.realizedDebtChangeUsdX18,
ctx.unrealizedDebtChangeUsdX18,
ctx.usdcCreditChangeX18,
ctx.wethRewardChangeX18
) = market.getVaultAccumulatedValues(
ud60x18(creditDelegation.valueUsd),
sd59x18(creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare),
sd59x18(creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare),
ud60x18(creditDelegation.lastVaultDistributedUsdcCreditPerShare),
ud60x18(creditDelegation.lastVaultDistributedWethRewardPerShare)
);
}
if (
ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()
&& ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero()
) {
continue;
}
vaultTotalRealizedDebtChangeUsdX18 = vaultTotalRealizedDebtChangeUsdX18.add(ctx.realizedDebtChangeUsdX18);
vaultTotalUnrealizedDebtChangeUsdX18 =
vaultTotalUnrealizedDebtChangeUsdX18.add(ctx.unrealizedDebtChangeUsdX18);
vaultTotalUsdcCreditChangeX18 = vaultTotalUsdcCreditChangeX18.add(ctx.usdcCreditChangeX18);
vaultTotalWethRewardChangeX18 = vaultTotalWethRewardChangeX18.add(ctx.wethRewardChangeX18);
Market::getVaultAccumulatedValues is called and it returns ctx.wethRewardChangeX18. The final line is the one to focus on as it adds ctx.wethRewardChangeX18 to vaultTotalWethRewardChangeX18 which represents the total weth reward change of the vault. This value is incorrect as I explained above as ctx.wethRewardChangeX18 is the total weth reward change for the market and not per vault.
Furthermore Vault::recalculateVaultsCreditCapacity is as follows:
function recalculateVaultsCreditCapacity(uint256[] memory vaultsIds) internal {
for (uint256 i; i < vaultsIds.length; i++) {
uint128 vaultId = vaultsIds[i].toUint128();
Data storage self = load(vaultId);
uint256 connectedMarketsConfigLength = self.connectedMarkets.length;
if (connectedMarketsConfigLength == 0) continue;
EnumerableSet.UintSet storage connectedMarkets = self.connectedMarkets[connectedMarketsConfigLength - 1];
uint128[] memory connectedMarketsIdsCache = new uint128[]());
updateVaultAndCreditDelegationWeight(self, connectedMarketsIdsCache);
(
uint128[] memory updatedConnectedMarketsIdsCache,
SD59x18 vaultTotalRealizedDebtChangeUsdX18,
SD59x18 vaultTotalUnrealizedDebtChangeUsdX18,
UD60x18 vaultTotalUsdcCreditChangeX18,
UD60x18 vaultTotalWethRewardChangeX18
) = _recalculateConnectedMarketsState(self, connectedMarketsIdsCache, true);
if (!vaultTotalRealizedDebtChangeUsdX18.isZero()) {
self.marketsRealizedDebtUsd = sd59x18(self.marketsRealizedDebtUsd).add(
vaultTotalRealizedDebtChangeUsdX18
).intoInt256().toInt128();
}
if (!vaultTotalUnrealizedDebtChangeUsdX18.isZero()) {
self.marketsUnrealizedDebtUsd = sd59x18(self.marketsUnrealizedDebtUsd).add(
vaultTotalUnrealizedDebtChangeUsdX18
).intoInt256().toInt128();
}
if (!vaultTotalUsdcCreditChangeX18.isZero()) {
self.depositedUsdc = ud60x18(self.depositedUsdc).add(vaultTotalUsdcCreditChangeX18).intoUint128();
}
if (!vaultTotalWethRewardChangeX18.isZero() && self.wethRewardDistribution.totalShares != 0) {
SD59x18 vaultTotalWethRewardChangeSD59X18 =
sd59x18(int256(vaultTotalWethRewardChangeX18.intoUint256()));
self.wethRewardDistribution.distributeValue(vaultTotalWethRewardChangeSD59X18);
}
(, SD59x18 vaultNewCreditCapacityUsdX18) =
_updateCreditDelegations(self, updatedConnectedMarketsIdsCache, false);
emit LogUpdateVaultCreditCapacity(
vaultId,
vaultTotalRealizedDebtChangeUsdX18.intoInt256(),
vaultTotalUnrealizedDebtChangeUsdX18.intoInt256(),
vaultTotalUsdcCreditChangeX18.intoUint256(),
vaultTotalWethRewardChangeX18.intoUint256(),
vaultNewCreditCapacityUsdX18.intoInt256()
);
}
}
In this function, the inflated vault::vaultTotalWethRewardChangeX18 variable is returned from vault::_recalculateConnectedMarketsState and is then used to call self.wethRewardDistribution.distributeValue(vaultTotalWethRewardChangeSD59X18);. See Distribution::distributeValue below:
function distributeValue(Data storage self, SD59x18 value) internal {
if (value.eq(SD59x18_ZERO)) {
return;
}
UD60x18 totalShares = ud60x18(self.totalShares);
if (totalShares.eq(UD60x18_ZERO)) {
revert Errors.EmptyDistribution();
}
SD59x18 deltaValuePerShare = value.div(totalShares.intoSD59x18());
self.valuePerShare = sd59x18(self.valuePerShare).add(deltaValuePerShare).intoInt256();
}
Distribution::distributeValue takes the inflated weth rewards returned in vault::vaultTotalWethRewardChangeX18 and distributes it between all shareholders which allows actors who hold shares of a particular vault to claim fees for the entire market which prevents actors with other vault shares connected to the same market from claiming their fees from the market.
Proof Of Code (POC)
function testFuzz_vaultinflatedwethrewards(
uint128 assetsToDepositA,
uint128 assetsToDepositB,
uint256 vaultId1,
uint256 vaultId2,
uint256 marketId1,
uint128 marketFees,
uint256 adapterIndex
)
external
{
vm.stopPrank();
VaultConfig memory fuzzVaultConfig1 = getFuzzVaultConfig(vaultId1);
vm.assume(fuzzVaultConfig1.asset != address(usdc));
PerpMarketCreditConfig memory fuzzMarketConfig1 = getFuzzPerpMarketCreditConfig(marketId1);
VaultConfig memory fuzzVaultConfig2 = getFuzzVaultConfig(vaultId2);
vm.assume(fuzzVaultConfig2.asset != address(usdc));
vm.assume(fuzzVaultConfig1.vaultId != fuzzVaultConfig2.vaultId);
uint256[] memory marketIds = new uint256[](2);
marketIds[0] = fuzzMarketConfig1.marketId;
marketIds[1] = fuzzMarketConfig1.marketId;
uint256[] memory vaultIds = new uint256[](2);
vaultIds[0] = fuzzVaultConfig1.vaultId;
vaultIds[1] = fuzzVaultConfig2.vaultId;
vm.prank(users.owner.account);
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
address userA = users.naruto.account;
address userB = users.sasuke.account;
uint128 assetsToDepositA =
uint128(bound(assetsToDepositA, calculateMinOfSharesToStake(fuzzVaultConfig1.vaultId), fuzzVaultConfig1.depositCap / 2));
fundUserAndDepositInVault(userA, fuzzVaultConfig1.vaultId, assetsToDepositA);
uint128 assetsToDepositB =
uint128(bound(assetsToDepositB, calculateMinOfSharesToStake(fuzzVaultConfig2.vaultId), fuzzVaultConfig2.depositCap / 2));
fundUserAndDepositInVault(userB, fuzzVaultConfig2.vaultId, assetsToDepositB);
vm.startPrank(userA);
marketMakingEngine.stake(fuzzVaultConfig1.vaultId, uint128(IERC20(fuzzVaultConfig1.indexToken).balanceOf(userA)));
vm.stopPrank();
vm.startPrank(userB);
marketMakingEngine.stake(fuzzVaultConfig2.vaultId, uint128(IERC20(fuzzVaultConfig2.indexToken).balanceOf(userB)));
vm.stopPrank();
marketFees = uint128(bound(marketFees, calculateMinOfSharesToStake(fuzzVaultConfig1.vaultId), fuzzVaultConfig1.depositCap / 2));
deal(fuzzVaultConfig1.asset, address(fuzzMarketConfig1.engine), marketFees);
vm.prank(address(fuzzMarketConfig1.engine));
marketMakingEngine.receiveMarketFee(fuzzMarketConfig1.marketId, fuzzVaultConfig1.asset, marketFees);
assertEq(IERC20(fuzzVaultConfig1.asset).balanceOf(address(marketMakingEngine)), marketFees);
if(fuzzVaultConfig1.asset != marketMakingEngine.workaround_getWethAddress()) {IDexAdapter adapter = getFuzzDexAdapter(adapterIndex);
vm.startPrank(address(fuzzMarketConfig1.engine));
marketMakingEngine.convertAccumulatedFeesToWeth(fuzzMarketConfig1.marketId, fuzzVaultConfig1.asset, adapter.STRATEGY_ID(), bytes(""));
vm.stopPrank();
}
uint256 totalwethrewardmarket = marketMakingEngine.workaround\_gettotalWethReward(fuzzMarketConfig1.marketId);
console.log(totalwethrewardmarket);
SD59x18 narutorewardamount = marketMakingEngine.exposed\_getActorValueChange(fuzzVaultConfig1.vaultId, bytes32(uint256(uint160(userA))));
console.log(narutorewardamount.unwrap());
SD59x18 sasukerewardamount = marketMakingEngine.exposed\_getActorValueChange(fuzzVaultConfig2.vaultId, bytes32(uint256(uint160(userB))));
console.log(sasukerewardamount.unwrap());
vm.startPrank(userA);
marketMakingEngine.claimFees(fuzzVaultConfig1.vaultId);
}
OPTIONAL ADDON THAT MAY BE NEEDED IF RUNNING INTO WORKAROUND ERRORS WHEN RUNNING POC
Note that I added the following workarounds to VaultHarness.sol and MarketHarness.sol to get values I needed and I may have used them for POC's so if some of the tests do not work due to workaround functions not being found, add the following functions to VaultHarness.sol:
function workaround_CreditDelegation_getweight(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.weight;
}
function workaround_Vault_getTotalCreditDelegationWeight(
uint128 vaultId
)
external view returns (uint128)
{
Vault.Data storage vaultData = Vault.load(vaultId);
return vaultData.totalCreditDelegationWeight ;
}
function workaround_CreditDelegation_getlastVaultDistributedRealizedDebtUsdPerShare(uint128 vaultId, uint128 marketId) external view returns (int128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare;}
function workaround_CreditDelegation_setvalueUsd(uint128 vaultId, uint128 marketId, uint128 valueUsd) external {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
creditDelegation.valueUsd = valueUsd;
}
function workaround_CreditDelegation_getlastVaultDistributedUnrealizedDebtUsdPerShare(uint128 vaultId, uint128 marketId) external view returns (int128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare;}
function workaround_CreditDelegation_getlastVaultDistributedUsdcCreditPerShare(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedUsdcCreditPerShare;}
function workaround_CreditDelegation_getlastVaultDistributedWethRewardPerShare(uint128 vaultId, uint128 marketId) external view returns (uint128) {
CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, marketId);
return creditDelegation.lastVaultDistributedWethRewardPerShare;}
Add the following functions to MarketHarness.sol:
function workaround_gettotalWethReward(uint128 marketId) external view returns (uint256) {
Market.Data storage market = Market.load(marketId);
return market.wethRewardPerVaultShare;
}
function workaround_getrealizedDebtUsdPerVaultShare(uint128 marketId) external view returns (int128) {
Market.Data storage market = Market.load(marketId);
return market.realizedDebtUsdPerVaultShare;
}
After registering the selectors of these functions in TreeProxyUtils.sol and increasing the bytes array size, it should work as expected and return the correct values
Impact
Overestimation of Rewards: If a vault has a small share of the total delegated credit, it will receive an inflated reward that does not correspond to its actual contribution.
Reward Theft: Malicious actors could create or manipulate vaults with minimal delegated credit to claim rewards intended for the entire market.
Loss of Funds: Legitimate users with larger shares of delegated credit may lose out on rewards they are entitled to, as these rewards are siphoned off by malicious actors.
Tools Used
Manual Review, Foundry
Recommendations
Include vaultCreditShareX18 Multiplication:
Align wethRewardChangeX18 with the other per-share calculations:
wethRewardChangeX18 = !lastVaultDistributedWethRewardPerShareX18.isZero()
? (
ud60x18(self.wethRewardPerVaultShare)
.sub(lastVaultDistributedWethRewardPerShareX18)
).mul(vaultCreditShareX18)
: UD60x18_ZERO;
This ensures that the vault only receives the portion of WETH that corresponds to its share of total delegated credit.
Add Zero-Check for Initial Distribution:
Mirror the pattern used for realized/unrealized debt and USDC credit so that if lastVaultDistributedWethRewardPerShareX18 == 0, the function returns zero for the vault’s WETH change (or handles first-time logic appropriately).
