# Lines of code https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingConfig.sol#L341-L1006 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAOConfig.sol#L641-L19710 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolsConfig.sol#L771-L10810 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/RewardsConfig.sol#L362-L10010 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/StableConfig.sol#L472-L15210 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L651-L964 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolMath.sol#L2101-L2283 # Vulnerability details # 1、The range and adjustment in functions are inconsistent with their notices in configs’ contract. ## Impact Inconsistencies between the notices and actual code for range and adjustment functions in multiple contracts, including StakingConfig, DAOConfig, PoolsConfig, RewardsConfig, StableConfig, and PriceAggregator, are causing confusion and potential financial losses. This stems from unclear standards and variations in how ranges are adjusted, leading to discrepancies between expected and actual outcomes. Manual calculations are often required, hindering user experience and increasing the risk of errors. To address this, a clear, unified standard for range and adjustment is urgently needed in both code and whitebook, with consistent implementation across all affected contracts. To remain the standard of `Range: x to x with an adjustment of x`, that is, adjusting the range with each adjustment iteration, including raising the upper limit and lowering the lower limit, one of the simplest approach is to replace `>` and `<` with `>=` and `<=`, respectively. In cases where `0` might be involved, adding `+1` to the lower limit parameters can address this issue. ## Proof of Concept In the `StakingConfig:: changeMaxUnstakeWeeks`, the notice of `Range: 20 to 108 with an adjustment of 8` implies that the range for maxUnstakeWeeks is from 20 to 80, with an adjustment of 8, and the upper limit can be adjusted to +8, resulting in 116, and the lower limit can be adjusted to -8, resulting in 22. However, based on the conditional statements in the function, `if (maxUnstakeWeeks < 108) maxUnstakeWeeks += 8`, and `if (maxUnstakeWeeks > 20) maxUnstakeWeeks -= 1`, the actual upper limit for `maxUnstakeWeeks` can be adjusted to 107 + 8 = 115, and the actual lower limit can be adjusted to 21 - 8 = 13. Consequently, the actual adjusted range in this function is from 13 to 115, but the notice indicates a adjusted range of 12 to 116, which means the difference of upper limit is 115 - 116 = -1 while the difference of lower limit is 13 - 12 = +1, making the standard of range and adjustment inconsistent and confused. Similar inconsistencies exist in the `changeMinUnstakePercent` and `changeModificationCooldown` functions of the contract. The notice for these functions suggests adjusted ranges of 5 to 55, 6 hours 15 minutes to 0 minutes, respectively. However, the actual adjusted ranges in the functions are 6 to 54, 6 hours 14 minutes to 1 minute, respectively. Furthermore, in the `changeMinUnstakeWeeks` function, there is a case where the notice's adjusted range matches the original range: the notice range is 1 to 12, and the adjusted range after modification is 0 to 13. However, the function' adjusted range remains 1 to 12, similar to its notice unadjusted range, according to `if (minUnstakeWeeks < 12) minUnstakeWeeks += 1` and `if (minUnstakeWeeks > 1) minUnstakeWeeks -= 1`. This may lead users to misunderstand that the standard is that within the range, there is adjustment, rather than the standard of adjusting the range with each adjustment iteration, including raising the upper limit and lowering the lower limit. Such inconsistencies between the notice and functions in range and adjustment can lead to confusion. Users may be uncertain about the standards for range and adjustment—whether it is the the standard A, for each adjustment within the range, the upper limit can be increased and the lower limit can be decreased, or whether it is the standard B, within the range, each adjustment is made. Even with the standard A, it is not intuitive and has variations, requiring manual calculations each time. Additionally, such inconsistencies are present in other contracts related to configuration, including DAOConfig.sol, PoolsConfig.sol, RewardsConfig.sol, StableConfig.sol, and PriceAggregator.sol. If calculations are required each time, it can negatively impact the user experience that the calculation errors mentioned can potentially lead to financial losses. [DAOConfig](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAOConfig.sol#L64C1-L197C10) [PoolsConfig](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolsConfig.sol#L77C1-L108C10) [RewardsConfig](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/rewards/RewardsConfig.sol#L36C2-L100C10) [StableConfig](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/StableConfig.sol#L47C2-L152C10) [PriceAggregator](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L65C1-L96C4) Therefore, in both the code and the whitepaper's parameters section, a clear, simple and unified standard for range and adjustment is needed. This would facilitate users' understanding and usage of these parameters, ultimately enhancing the user experience and finance. To remain the standard A of adjusting the range with each adjustment iteration, including raising the upper limit and lowering the lower limit, one of the simplest approach is to replace `>` and `<` with `>=` and `<=`, respectively. In cases where `0` might be involved, adding `+1` to the lower limit parameters can address this issue. This adjustment can be applied consistently across all the six contracts, as shown in the example of `StakingConfig` contract below: [Whitepaper: Parameters](https://docs.salty.io/decentralization/parameters) [StakingConfig::changeMinUnstakeWeeks, changeMaxUnstakeWeeks, changeMinUnstakePercent, changeModificationCooldown](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingConfig.sol#L34C1-L100C6) ```solidity // Range: 2 to 12 with an adjustment of 1 function changeMinUnstakeWeeks(bool increase) external onlyOwner { if (increase) { - if (minUnstakeWeeks < 12) minUnstakeWeeks += 1; + if (minUnstakeWeeks <= 12) minUnstakeWeeks += 1; } else { - if (minUnstakeWeeks > 1) minUnstakeWeeks -= 1; // @audit: To prevent the occurrence of 0 values, add 1 to the lower limit. + if (minUnstakeWeeks >= 2) minUnstakeWeeks -= 1; } emit MinUnstakeWeeksChanged(minUnstakeWeeks); } // Range: 20 to 108 with an adjustment of 8 function changeMaxUnstakeWeeks(bool increase) external onlyOwner { if (increase) { - if (maxUnstakeWeeks < 108) maxUnstakeWeeks += 8; + if (maxUnstakeWeeks <= 108) maxUnstakeWeeks += 8; } else { - if (maxUnstakeWeeks > 20) maxUnstakeWeeks -= 8; + if (maxUnstakeWeeks >= 20) maxUnstakeWeeks -= 8; } emit MaxUnstakeWeeksChanged(maxUnstakeWeeks); } // Range: 10 to 50 with an adjustment of 5 function changeMinUnstakePercent(bool increase) external onlyOwner { if (increase) { - if (minUnstakePercent < 50) minUnstakePercent += 5; + if (minUnstakePercent <= 50) minUnstakePercent += 5; } else { - if (minUnstakePercent > 10) minUnstakePercent -= 5; + if (minUnstakePercent >= 10) minUnstakePercent -= 5; } emit MinUnstakePercentChanged(minUnstakePercent); } // Range: 16 minutes to 6 hours with an adjustment of 15 minutes function changeModificationCooldown(bool increase) external onlyOwner { if (increase) { - if (modificationCooldown < 6 hours) modificationCooldown += 15 minutes; + if (modificationCooldown <= 6 hours) modificationCooldown += 15 minutes; } else { - if (modificationCooldown > 15 minutes) modificationCooldown -= 15 minutes; // @audit To prevent the occurrence of 0 values, add 1 to the lower limit. + if (modificationCooldown >= 16 minutes) modificationCooldown -= 15 minutes; } emit ModificationCooldownChanged(modificationCooldown); } ``` ## Tools Used Manual Review ## Recommended Mitigation Steps To remain the standard of `Range: x to x with an adjustment of x`, that is, adjusting the range with each adjustment iteration, including raising the upper limit and lowering the lower limit, one of the simplest approach is to replace `>` and `<` with `>=` and `<=`, respectively. In cases where `0` might be involved, adding `+1` to the lower limit parameters can address this issue as shown in Proof of Concept. # 2. Omitting the equal condition in the `PoolMath:: _determineZapSwapAmount` ## Impact Omitting the condition `zapAmountA * reserveB == reserveA * zapAmountB` in the `_determineZapSwapAmount` function would result in incorrect swap calculations when the ratios of zapAmountA/zapAmountB and reserveA/reserveB are equal. ## Proof of Concept `if the condition zapAmountA * reserveB == reserveA * zapAmountB` was not considered in the `_determineZapSwapAmount function` in the `PoolMath` contract. Without this condition, the function would fail to handle cases where the condition zapAmountA * reserveB and reserveA * zapAmountB are exactly equal, which would lead to incorrect swap amounts. The function's logic relies on the two if statements to determine which asset to swap based on the condition. However, if it is equal, neither if condition would be met, and the function would likely return incorrect swap amounts, potentially leading to unintended financial consequences. Here's a recommendation to change the code to address the missing condition [PoolMath::_determineZapSwapAmount](https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolMath.sol#L210C1-L228C3) ```solidity function _determineZapSwapAmount( uint256 reserveA, uint256 reserveB, uint256 zapAmountA, uint256 zapAmountB ) internal pure returns (uint256 swapAmountA, uint256 swapAmountB ) { // zapAmountA / zapAmountB exceeds the ratio of reserveA / reserveB? - meaning too much zapAmountA - if ( zapAmountA * reserveB > reserveA * zapAmountB ) + if ( zapAmountA * reserveB >= reserveA * zapAmountB ) (swapAmountA, swapAmountB) = (_zapSwapAmount( reserveA, reserveB, zapAmountA, zapAmountB ), 0); // zapAmountA / zapAmountB is less than the ratio of reserveA / reserveB? - meaning too much zapAmountB if ( zapAmountA * reserveB < reserveA * zapAmountB ) (swapAmountA, swapAmountB) = (0, _zapSwapAmount( reserveB, reserveA, zapAmountB, zapAmountA )); // Ensure we are not swapping more than was specified for zapping if ( ( swapAmountA > zapAmountA ) || ( swapAmountB > zapAmountB ) ) return (0, 0); return (swapAmountA, swapAmountB); } } ``` ## Tools Used Manual Review ## Recommended Mitigation Steps It is recommended to add the equal condition to the if statement as shown in the Proof of Concept: ``` solidity - if ( zapAmountA * reserveB > reserveA * zapAmountB ) + if ( zapAmountA * reserveB >= reserveA * zapAmountB ) ``` ## Assessed type Math


Code4rena	NOYA	NOYA	26/04/2024	17/05/2024	21 days	$65,000 in USDC + NOYA stars	48/152	$125.31	1	1	0	0	Awarded
Code4rena	DYAD	DYAD	18/04/2024	25/04/2024	7 days	$36,500 in USDC	25/183	$460.80	1	0	1	0	Completed
Code4rena	Salty.IO	Salty.IO	16/01/2024	30/01/2024	14 days	$80,000 in USDC	150/178	$11.69	1	0	0	-	Completed