# StrategyRouterLib.sol bug when subtracting from balance Medium · Three Sigma · DeFi infrastructure · 10th November, 2023 Finding 3S-Clip Finance-M06 of the Clip Finance Strategies security review. - Protocol: https://www.clip.finance/ - Report: /reports/clip-finance-i - Source: https://cdn.sanity.io/files/qoqld077/production/c23d04c8223879d2443221caf3ccb55ac118441a.pdf --- ### Description [Lines 665 and 666](https://github.com/ClipFinance/strategy-router/blob/master/contracts/StrategyRouterLib.sol#L666) of the StrategyRouterLib have the following code: currentTokenDatas[j].currentBalance -= desiredAllocationUniform; currentTokenDatas[j].currentBalanceUniform -= desiredAllocationUniform; // not actually in uniform format Here, variable desiredAllocationUniform actually holds the original desired allocation value (not in the uniform format), so in line 666 the subtraction is being performed by variables in different formats. Depending on the difference between the original and the uniform decimals, one of the three scenarios should happen: - if the number of decimals in the same, the code should run without a problem - if the number of original decimals is higher than the number of uniform decimals, the execution will most likely underflow and revert in line 666. - if the number of original decimals is smaller than the number of uniform decimals, the contract will try to transfer tokens it does not have and revert. - no under/overflow happens but the accounting becomes incorrect. ### Recommendation ```solidity Change line 666 to currentTokenDatas[j].currentBalanceUniform -= toUniform(desiredAllocationUniform, supportedTokensWithPrices[j].token); ``` ### Status Addressed in [#ed8e3c9](http://ed8e3c994f013eb9af523201ff97df5f43e34baf) --- Related findings: - [`BaseTOFT.sol`: `retrieveFromStrategy` can be used to manipulate other user's positions due to absent approval check](https://0xsimao.com/findings/tapioca-dao-retrieve-manipulate-absent-approval): Tapioca DAO - [Users will steal excess funds from the Vault due to `VaultPoolLib::redeem()` not always decreasing `self.withdrawalPool.raBalance` and `self.withdrawalPool.paBalance`](https://0xsimao.com/findings/cork-protocol-steal-excess-redeem-withdrawal): Cork Protocol - [_checkNoBalanceChange(...) cycle should break in BaseRouter](https://0xsimao.com/findings/fuji-finance-balance-change-cycle-break): Fuji Finance - [Swap amount can be 0 as it divides the balance by 2, DoSing ShadowStrategyGauge::collectGaugeRewards()](https://0xsimao.com/findings/yieldoor-i-divides-sing-shadow-rewards): Yieldoor Gauges