Full Report
The Financial Non-Fungible Token (FNFT) of Revest has vaults that allow the transferring of access to these vaults. This works by a user sending some asset, such as wETH to the contract. Then, using the funds from that asset, FNFTs are generated with an identifier. A user can divide up the FNFTs as they like. Once the vault is unlocked, then the FNFTs can be burned to receive funds from the underlying asset, such as wETH. The entry contract has three interfaces to mint FNFTs by locking previous assets. mintTimeLock locks the asset for a set period of time, mintValueLock will unlock only when the asset goes above or below a certain value and mintAddressLock will unlock assets when a specific account opens it. All of these paths call the same set of additional contracts for unlocking and locking contracts. An additional piece of functionality called depositAdditionalToFNFT is used to lock more underlying assets to an existing lock. When this happens, it assumes that the amount can be evenly distributing among the existing NFT owner. If it is not, then a new lock is placed on the ID and burns some quantity of the old FNFT tokens and mints the new quantity. This is done by adding up the old quantity with the new quantity to get a total quantity. So, what's the vulnerability then? Well, there is two. The simpler bug (and one discovered by the authors of the post) is a logic issue. When handling multiple deposits on the same call, the depositAmount for a given FNFT is put BOTH into the old lock and the new lock! This means we can use our funds twice, which is a recipe for disaster. Instead, it should destroy the original lock and put the funds into the new one only. The original vulnerability comes from a re-entrancy attack. When creating or updating the FNFT, the id gets updated. By creating an FNFT, the id should be increased. However, prior to incrementing the counter the variable fnftId, ERC-1155 standard's callback mechanism can be triggered. Since the id has not been incremented in the state yet, shenanigans can be pulled. Remember how the update will either add the funds to a new FNFT (and increment the ID) or use the current one? By reentering the contract with the fnftId in the wrong state, we can create a update the previous FNFT (increment by 1) to update the depositAmount of a token. Why is this bad? This state update is catastrophic because it will update the funds in the contract but not the amount of tokens minted. As a result, an attacker can create a large amount of tokens with zero value (token 2) and update the value of token 1 to really update the value of token 2! By doing this, the cost of token 2 is not zero but there are a ton of tokens we can withdraw for a high value. That last re-entrancy vulnerability is insanely complex and I cannot believe somebody truly figured that out. The diagrams in the blog post are amazing for explaining this, as there are many steps to make this work.
Analysis Summary