6.1 Incorrect rounding favors withdrawing users
When a user withdraws a specific amount of assets ( _amount) from the vault, the _withdrawRequest(...) function calculates the corresponding number of shares to burn. This calculation uses standard integer division, which implicitly rounds the result down.
function _withdrawRequest(address _user, uint256 _amount, address _validator)
private
returns (uint256 shareDecreaseUser, uint256 unbondNonce)
{
// ...
} else {
// calculate share decrease
// @audit-issue The result of this division is rounded down.
shareDecreaseUser = (_amount * globalPriceDenom * WAD) / globalPriceNum;
}
// ...
_burn(_user, shareDecreaseUser);
// ...
}
By rounding down, the contract burns slightly fewer shares than are required to cover the withdrawn assets. This rounding error consistently benefits the withdrawing user at the expense of all remaining liquidity providers in the vault. Over a large number of withdrawal transactions, these small discrepancies can accumulate, leading to a gradual drain of value from the protocol.
An evil user could inflate the share price to make the rounding error bigger and execute multiple withdrawals without spending shares, effectively draining par of the assets of the vault.
Recommendation
Consider changing the rounding direction when calculating shares to burn during a withdrawal. The calculation should round *up* to ensure that the user provides a sufficient amount of shares to cover the assets they are taking.
Resolution
This has been fixed in branch pol-fix-epsilon, where we remove the epsilon value that was making it possible for the contract to favor users during withdrawals.
6.2 No mechanism to reflect slashing events
The TruStakePOL contract tracks the amount of POL staked with each validator using the stakedAmount field within the Validator struct. This value is a critical component of the totalStaked() calculation, which in turn determines the vault's share price.
The stakedAmount is updated only through internal actions such as staking ( _stake), unbonding ( _unbond), and restaking rewards ( _restake). The protocol lacks a mechanism to verify if this internally tracked amount matches the actual balance of staked POL held by the external ValidatorShare contracts.
struct Validator {
ValidatorState state;
// @audit-issue This value is trusted internally but not verified against the external reality.
uint256 stakedAmount;
address validatorAddress;
}
If a validator is slashed, its actual staked POL balance will decrease. However, the stakedAmount variable in the TruStakePOL contract will not be updated to reflect this loss.
This discrepancy leads to an overestimation of the vault's total assets. The totalStaked() function will return an inflated value, causing the sharePrice() to be calculated incorrectly as higher than it should be. The vault will become undercollateralized, where the value of the issued shares exceeds the value of the underlying assets. This situation benefits savvy users who withdraw early, as they can redeem their shares for more POL than they are worth, at the expense of all other users. The last users to withdraw may find that there are insufficient assets left to cover their shares, resulting in a loss of funds.
Additionally, calls to buyVoucherPOL(...) and sellVoucher_newPOL(...) require the share price from the ValidatorShare to be at least one, which may not be the case in case of slashing, affecting deposit and withdrawals mechanisms.
Recommendation
Consider implementing a mechanism to account for slashing events.
Resolution
Slashing is currently disabled on Polygon, with no confirmed timeline for activation. While there is some preliminary code in the validator contracts to support slashing, its final implementation, parameters, and enforcement mechanisms remain undefined. As such, this issue is not currently actionable.
6.3 Unbounded loop over validators may lead to denial of service
The contract owner has the ability to add new validators to the system via the addValidator(...) function, which appends the new validator's address to the _validatorAddresses storage array. While validators can be disabled using disableValidator(...), there is no mechanism to remove them from this array.
Several core functions, including totalStaked(), totalRewards(), and _restake() (called by compoundRewards(...)), iterate over the entire _validatorAddresses array to perform their calculations.
function totalStaked() public view override returns (uint256) {
TruStakePOLStorageStruct storage $ = _getTruStakePOLStorage();
uint256 validatorCount = $._validatorAddresses.length;
uint256 stake;
// @audit-issue This loop's cost grows with the number of validators.
for (uint256 i; i < validatorCount; ++i) {
stake += $._validators[$._validatorAddresses[i]].stakedAmount;
}
return stake;
}
The issue is that the _validatorAddresses array can grow indefinitely. As more validators are added over time, the gas cost of executing these functions will increase linearly. Eventually, the transaction cost could exceed the block gas limit, rendering these critical functions unusable.
This would create a permanent Denial of Service (DoS), with the following impacts:
compoundRewards(...) would become uncallable, preventing all rewards from being restaked.totalStaked() and totalRewards() would revert, breaking the sharePrice() calculation and thus all deposit and withdrawal operations.
Recommendation
Consider introducing a mechanism to remove validators from the _validatorAddresses array. When a validator is no longer needed, it should be fully decommissioned and removed.
Resolution
While we acknowledge the concern, this loop is not expected to pose a practical risk in our specific context. The Polygon validator set is capped at 105 validators, and at TruFin, we operate with a tightly controlled validator onboarding process. All validators are KYC’d and whitelisted, and we expect to onboard only a small, static subset of the available set. As such, the number of active validator entries will remain low and predictable, ensuring the gas cost of looping operations remains well within safe bounds.
6.4 maxWithdraw calculation ignores per-validator liquidity constraints
The maxWithdraw(...) function is a view function usually intended to inform users of the maximum amount of POL they can withdraw in one call. The withdrawal logic, handled by _withdrawRequest(...), correctly enforces that a withdrawal from a specific validator cannot exceed the amount staked with that validator.
However, the maxWithdraw(...) function calculates the withdrawable amount based on the user's total share holdings, without considering how the underlying liquidity is distributed across the various validators.
function maxWithdraw(address _user) public view override returns (uint256) {
// @audit This calculation is based on the user's total shares.
uint256 preview = previewRedeem(balanceOf(_user));
if (preview == 0) return 0;
TruStakePOLStorageStruct storage $ = _getTruStakePOLStorage();
// @audit-issue It does not account for the stake on any single validator.
return preview + $._epsilon;
}
This creates a discrepancy that can affect user's experience. A user might check maxWithdraw(...), see a large available amount, and attempt to withdraw it using the withdraw(...) function, which targets the default validator. If the default validator's staked amount is less than the user's requested amount, the transaction will revert with a WithdrawalAmountAboveValidatorStake error, even though the user was led to believe the withdrawal was possible. This can cause user confusion and wasted gas on failed transactions.
Recommendation
Consider modifying the maxWithdraw(...) function to provide more accurate information. The function could accept a validator address as an input parameter. This would allow it to return the maximum amount a user can withdraw from a *specific* validator, aligning its logic with that of the withdraw(...) and withdrawFromSpecificValidator(...) functions and preventing misleading results.
Resolution
The maxWithdraw(...) function is designed to reflect the theoretical maximum a user can redeem based on their LST token balance, not validator-specific liquidity constraints. The per-validator limits are intentionally enforced at the withdrawal level and surfaced through the front end, which directs withdrawals to appropriate validators. As such, while the function may overstate the amount withdrawable from a specific validator, this is by design and does not impact protocol safety.
