First would be key to note that protocol is to support different types of ERC20, which include tokens that apply fee whenever being transferred, this can be seen from the ### ERC20 token behaviours in scope section of the audits README here.
https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L122-L134
This function allows a dNFT owner to deposit collateral into a vault, it first transfers the amount from the caller and then deposits it to the vault with the function here.
The issue with this implementation is that it could get very faulty for some assets, thats to say for tokens that remove fees whenever they are being transferred, then protocol is going to have an accounting flaw as it assumes the amount of assets sent with this line: _vault.asset().safeTransferFrom(msg.sender, address(vault), amount); is whats being received. It would be key to note that the impact of this varies on the logic of the asset in some cases this could be just fees that might amount to little value which would still be considered as a leak of value.
However in some cases, for some tokens such as the cUSDCv3, it contains a special case for when the amount to be deposited == type(uint256).max in their transfer functions that result in only the users balance being transferred. This can easily be used to exploit or rug pull the vault depending on the context, as in the vault during deposits, this execution id2asset[id] += amount would assume amount to be type(uint256).max and depositor can then withdraw as much tokens as they like in consequent withdrawals, breaking the intended rate of redemption 1 DYAD to always be $1, as they are no collaterals to back this.
Also the protocol has stated that among tokens to consider as collaterals, we should consider LSTs and LRTs; however, even these set of tokens are vulnerable to this, from LSTs that support positive/negative rebases to LSTs that have 1-2 wei corner cases when querying their transfers.
As explained in the last section, this could lead to multiple bug cases depending on the specific fee/transfer logic applied to the asset, but in both cases this leads to a heavy accounting flaw and one might easily game the system by coupling this with the minting/withdrawal logic.
Keep in mind that protocol already integrates Lidos WSTETH on the mainnet, but since Chainlink does not have a specific feed for WSTETH/USD on the mainnet, protocol uses the Chainlink mainnets STETH/USD feed instead as seen here, since this is not for the asset used as collateral. The protocol would decide to change and decide to integrate into protocol STETH directly instead which still opens them up to this issue, specifically the 1-2 wei corner case, otherwise this bug case is still applicable to other.
Additional note to judge: This finding was first submitted as a H/M finding and then withdrawn after the scope of tokens to consider were refactored; however, going through the docs and information passed by sponsors we can see that protocol plans on integrating LSTs and LRTs and in that case the 1-2 wei corner case is applicable to LSTs which still makes protocol vulnerable to bug case, so consider upgrading.
Do not support these tokens; alternatively a pseudo fix would be to only account for the difference in balance when receiving tokens, but this might not suffice for all tokens considering the future integration of LSTs/LRTs that have rebasing logic attached to them.
This bug case is applicable to attempts of safeTransfer/safeTransferFrom in scope, and can be pinpointed using these search commands
Main focus should be in the instances where these tokens are being deposited to protocol.
