On 24/08 Trust Security disclosed a variety of DOS issues to 30+ projects through Immunefi and private bug bounty programs. In total $50k from 15 projects were paid out. We'll start with a technical description of the issue for those of you who like to get straight to business. The second section will be musings around bug causality theory. Finally, we'll break down how each project responded and give them a rating accordingly.
Description
EIP-2612 defined the highly popular ERC20 extension, Permit. It transfers the burden of holding native (gas) tokens away from users, by allowing them to sign an approval off-chain and send it to a trusted service, which could use the funds as if the user called approve(). It operates by a crypto signature of the following fields:
owner (user)
spender (trusted address)
value (amount)
nonce (an int that increases after every permit executes)
deadline
When permit() executes, the key things it checks are:
The sig is valid (compares it to a sig constructed using parameters + user's nonce)
Deadline hasn't passed.
Note that by design, the token ignores the msg.sender of the permit() call. Combined with the fact TXs can be observed in the mempool (by anyone, or at least by the sequencer in some L2s), it means that a permit() can be easily frontran (simply duplicate the TX arguments). But that's no big deal, as the EIP says, because:
"Though the signer of a Permit may have a certain party in mind to submit their transaction, another party can always front run this transaction and call permit before the intended party. The end result is the same for the Permit signer, however."
Is it though? There was something that felt off with the concession that anyone could make another TX revert. Indeed, when permit() is a standalone (external) TX, not much can go wrong. Consider, though, a situation where permit() is part of a contract call:
This function deposits in a staking contract on behalf of the user. But what if an attacker extracts the _signature parameters from the deposit() call and frontruns it with a direct permit() ? In this case, the end result is harmful, since the user loses the functionality that follows the permit().
In fact, any function call that unconditionally performs permit() can be forced to revert this way. In case there is a fallback code path (using direct user approval), the DOS is short-term, as eventually the user / dApp would switch to using an alternative. Otherwise, the DOS is long-term (there are bypasses through flashbots in some chains, but that's a really bad scenario to resort to).
Below is an example of a reasonable workaround (code available here):
The code first attempts to call permit(), then catches an exception and would allow logic to continue in case the allowance is enough (this would be the case after a frontrun).
Functions like the vulnerable deposit() above were found in 100+ codebases. We proceeded to reach out to those with a bug bounty and had a variety of responses. These will be covered in the last section.
Root cause and bug theory
The existence and proliferation of this bug made us think quite a bit about how it came about. Eventually 100proof came up with this formal description:
You've got some action A that can be front-run. The EIP says that A; A* (where A* means "A reverted") is fine since it results in the same state change on-chain.
But in fact, A could be part of a sequence, e.g. A; B; C. And the problem is that A; (A; B ;C)* does not result in the same state change as A; B; C on-chain.
From this definition, we were able to generalize and see additional attack scenarios fitting this template:
delegateBySig() - From the ERC20 Governance module. It delegates on behalf of a token holder. Any composition with this function can potentially be griefed. Shoutout to Arbitrum for finding it!
Some ecosystem-specific EIP712 signature functions, which have different call paths.
It seems the original sin of this class of issues is that the signature describes the action desired, but lacks intent. When allowance is approved to a contract, it is usually for a specific purpose. Suppose the user signature would also cover an intent field and an on/off bit, and the nonce is only increased when the permission bit is flipped. Then, the user's signature could be safely accepted inside the permit() multiple times, as long as the user doesn't want to flip permissions. It would require a separate has_used bit in storage to make sure only first invocation actually increases allowances. This type of implementation would be much more robust and also only allow usage of allowance in a specific code path the user has explicitly consented.
The issue is a great example of how important it is to be security-focused when defining widely used standards. On the flip side, it is also an example of how diving into older EIPs and looking for hidden assumptions is a great way to find many bugs at once.
We're sure there are other subclasses of this bug family which don't involve signatures but we haven't gotten down that rabbit hole yet.
Bounty summary
The Immunefi Severity Standard assigns this attack vector Medium severity:
"Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol): Griefing is when the attacker calls certain functions of the smart contract that would put it in a suboptimal state, thus blocking normal function execution for any user. This would cause the user to lose money for sending the transaction, but when the smart contract is back to normal, the user would be able to call the function once again to complete it. In this instance, the attacker damaged the user by requiring them to send another transaction. The attacker does not profit, but they do damage the users or the protocol."
Open Zeppelin 💯
Rating | 4/5⭐ |
Reward | $5K |
Response time | 4 hours |
Mediation | - |
Notes | Issue is with safePermit(), deprecated due to this finding. Initially marked as Med despite DOS marked as critical (custom bounty). Negotiated to high and good will shown. |
AAVE 💯
Rating | 4.5/5⭐ |
Reward | $5K |
Response time | 7 days |
Mediation | - |
Notes | Migrated BBP to Immunefi |
The Graph 💯
Rating | 5/5⭐ |
Reward | $20K |
Response time | 5 days |
Mediation | - |
Notes | Assisted the team with private mitigation review which uncovered additional issues |
Uniswap-V2💯
Rating | 4.5/5⭐ |
Reward | $1k |
Response time | 5 days for initial response, 3 weeks resolution. |
Mediation | - |
Notes | Private BBP. Not in scope but awarded good will payout. |
Ribbon
Rating | 2.5/5⭐ |
Reward | - |
Response time | 4 days |
Mediation | Lost, susceptibility to frontrunning OOS |
Notes | Scope excludes FR under "congestion and scalability", which in our opinion is not the case. 3 assets affected. |
Pods
Rating | 3/5⭐ |
Reward | $1k * 2 assets |
Response time | 10 days |
Mediation | Won |
Notes | Used several dishonest arguments to try reducing severity. |
Nexus Mutual
Rating | 2/5⭐ |
Reward | - |
Response time | 2 hours |
Mediation | Won. Won't fix. |
Notes | Used several dishonest arguments to try reducing severity. |
Mars
Rating | 1/5⭐ |
Reward | - |
Response time | 2 days |
Mediation | Won. Refused to pay. |
Notes | Used devious tricks to refuse pay. Then offered a $200 payout to not share this with the community. |
Gro
Rating | 1/5⭐ |
Reward | - |
Response time | 2 days |
Mediation | Won. Refused to pay. |
Notes | Malicious project. |
Ease
Rating | 1/5 ⭐ |
Reward | - |
Response time | 2 days |
Mediation | Won. Won't fix. |
Notes | Used malicious arguments and ignored Immunefi mediation |
Kyber
Rating | 3.5/5 ⭐ |
Reward | $2k |
Response time | 2 days, then 8 days post mediation |
Mediation | Won |
Notes | Immunefi assisted in discussion with project |
DeBridge 💯
Rating | 4.5/5 ⭐ |
Reward | $5k |
Response time | 4 days |
Mediation | - |
Notes | Initially suggested Low bounty but quickly lined up to Med. |
SpookySwap 💯
Rating | 5/5 ⭐ |
Reward | $3k |
Response time | 9 days |
Mediation | - |
Notes |
Angle
Rating | 2.5/5 ⭐ |
Reward | $500 good will payout for 2 assets |
Response time | 7 days |
Mediation | - |
Notes | Won't fix. Users affected. |
Morpho 💯
Rating | 5/5 ⭐ |
Reward | - |
Response time | < 1 day |
Mediation | - |
Notes | Known issue, linked to proof |
Exactly
Rating | 1.5/5 ⭐ |
Reward | - |
Response time | < 1 day |
Mediation | Won. Won't fix. |
Notes | Used malicious arguments and ignored Immunefi mediation |
Brahma
Rating | 3.5 / 5⭐ |
Reward | $1.5k |
Response time | 6 days |
Mediation | Won |
Notes |
Arcade 💯
Rating | 5 / 5⭐ |
Reward | $1k |
Response time | 4 hours |
Mediation | - |
Notes | Custom impact table |
Goldfinch
Rating | 2 / 5⭐ |
Reward | $1k total for 3 assets |
Response time | 6 days |
Mediation | - |
Notes | Table states up to $5k per asset. Initially offered $250 total. |
WardenSwap
Rating | 3 / 5⭐ |
Reward | $1k |
Response time | 7 days. No action for 4.5 months, then paid. |
Mediation | Won. |
Notes | Initially argued for OOS. |
SushiSwap
Rating | 4 / 5⭐ |
Reward | $2k |
Response time | 15 days |
Mediation | - |
Notes | - |
Paraspace / ParallelFi
Rating | 1 / 5⭐ |
Reward | - |
Response time | 70 days |
Mediation | Won. Won't fix. |
Notes | Used unreasonable arguments, was not responsive at all. |
Centrifuge
Rating | 1 / 5⭐ |
Reward | - |
Response time | - |
Mediation | - |
Notes | Private BBP. Got no reasonable response to date. |
Premia
Rating | 1 / 5⭐ |
Reward | - |
Response time | - |
Mediation | - |
Notes | Private BBP. Haven't received any response to date. |
Summary
We have to commend and praise projects who have shown good will and support for the white hat community. However there were many projects who were giving "how do we invalidate or pay as little as possible for this finding?" vibes, and that ought to stop. Respect for white hats and appreciation of their time directly translates to respect for users' safety. A big picture view shows a strong correlation between a project's scale and reputation, and their general attitude towards reports. Our suggestion for hunters is to concentrate efforts on high-profile projects with reputation to lose.