Full Report
Deri is a derivatives protocol on various EVM platform. Users can add/remove margin, trade and so other functionality through the Gateway contract. When removing margin, the users calls the requestRemoveMargin() it emits an event for a bot to see. Once the bot sees it, it will call finishRemoveMargin() with signed event data and a signature to finalize the request. In total, there are three finishing calls: finishRemoveMargin, finishUpdateLiquidity and finishLiquidate. In the former two, they have an internal function for checking the _checkRequestId to increment the nonce to prevent replay attacks. This crucial check is missing from the function finishLiquidate(). However, since the position NFT would have already been burned then it would have failed anyway. So, no issue, right? What if we called one of the other functions? When decoding the information that is signed, there is NO check that the information is going to the proper function. When doing the actual value decoding, the original signed data has less information which I would have assumed a revert for not having enough data. The first two values are the same. But, the cumlativePnlOnEngine field in the liquidate struct matches the requiredMargin field. Since the verification happened in the previous call, there is no validation on any of these values! The liquidation value is scaled up to an insane amount. Additionally, other parts of the protocol have key invariants broken, allowing for more attacks. This was estimated to have a lost funds of 500K-1.5M, but with a bug bounty of only 10K, they reported the bug and moved on. The signatures had two bad flaws. First, the missing nonce increment. Second, the signature could be used on more than one call. I was also surprised that the abi.decode() didn't fail with the different data lengths. Overall, good finding with a fun write up!
Analysis Summary