top of page

Permission denied - The story of an EIP that sinned


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:

  1. owner (user)

  2. spender (trusted address)

  3. value (amount)

  4. nonce (an int that increases after every permit executes)

  5. deadline

When permit() executes, the key things it checks are:

  1. The sig is valid (compares it to a sig constructed using parameters + user's nonce)

  2. 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.


0 comments

Comments


bottom of page