# Wrong tokensToCheck logic in BaseRouter enables attackers to steal funds Crit/High · Three Sigma · Lending aggregator · 6th May, 2023 Finding 3S-FUJI-C02 of the Fuji Finance security review. - Report: /reports/fuji-finance - Source: https://cdn.sanity.io/files/qoqld077/staging/32181a28eac3175d15fb8924d249bb0d91ca350c.pdf --- ### Description The BaseRouter implements a mechanism to prevent tokens from being stuck that checks the balance of the router before and after the actions and ensures it remains equal. Problem is, the _addTokenToList(...) function has a bug such that only the last token interacted with is added to the array at the first element (0). The _isInTokenList(...) function returns index 0 if the token is not in the array, so the position uint256 position = index == 0 ? index : index + 1; will always be 0. Attackers can use this to steal other users by waiting for withdraw approvals for the BaseRouter of other users and then use these approvals to withdraw to the BaseRouter, followed by a deposit action in another vault to remove the asset of the withdraw from the tokensToCheck array. The asset is then held by the BaseRouter, which can be retrieved in a following transaction by the attacker by 1) depositing the funds to the vault, where the receiver and owner are the BaseRouter. 2) withdrawing these funds to the attacker, where the receiver is the attacker and the owner the BaseRouter. 3) depositing another asset to another vault, removing the first asset from the tokensToCheck array. ### Recommendation Fix the implementation of the _addTokenToList(...) function by keeping a memory variable with the number of tokens to check. The position to add in _addTokenToList(...) should be the number of tokens to check + 1, if the token is not in the array. ### POC https://github.com/threesigmaxyz/fuji-issues-external/blob/master/test/POC/POCWrongTokensToCheckLogic.t.sol#L80 ### Status Addressed here: [Fujicracy/fuji-v2#622](https://github.com/Fujicracy/fuji-v2/pull/622) --- Related findings: - [Attackers will steal the reserve from the `Vault` by receiving `ra` in `FlashSwapRouter::__swapDsforRa()`](https://0xsimao.com/findings/cork-protocol-steal-receiving-flash-dsfor): Cork Protocol - [Tokens with callbacks may allow malicious attackers to steal the protocol](https://0xsimao.com/findings/clip-finance-i-callbacks-malicious-attackers-steal): Clip Finance Strategies - [Fixed term loans can be deployed with a wrong fee manager and possibly steal all funds.](https://0xsimao.com/findings/maple-finance-iii-loans-deployed-fee-steal): Maple Finance - [Attackers can include other users nullifiers to make their funds stuck when adding liquidity to curve](https://0xsimao.com/findings/singularity-include-nullifiers-stuck-curve): Singularity