OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.81k stars 11.77k forks source link

OpenZeppelin avoided paying the bug bounty for disclosing a flaw in the contract that caused a freeze of $1.1B worth of assets #4474

Closed Dexaran closed 1 year ago

Dexaran commented 1 year ago

I would like to escalate this issue: https://twitter.com/Dexaran/status/1682826099800125442

Long story short:

I kindly request an explanation of the following:

  1. This bug bounty page says that "https://github.com/OpenZeppelin/openzeppelin-contracts" are in scope. So is https://github.com/OpenZeppelin/openzeppelin-contracts/tree/master/contracts/token/ERC20 in scope then? It looks like it is.

OpenZeppelinBugbounty6

  1. Permanent freeze of funds is considered a critical security vulnerability. I have a proof-of-concept script that demonstrates a permanent freeze of funds amounting in $1,1B worth of tokens. Isn't it true?

OpenZeppelinBugbounty7

  1. Your response states that "it is not a security bug report because it corresponds to the expected behavior to ERC-20 standard". This is not actually true because what I reported is a vulnerability in your particular implementation of the standard. The code is located here: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol If we will take this code and compile it "as is" without applying any changes - it will contain a vulnerability that has critical severity according to your own description at the bug bounty page: it can result in permanent freeze of funds for end users. And it already resulted in a freeze of $1,1B worth of tokens in 50 examined contracts out of 1200 existing contracts. Your code contains this flaw. And this code is in scope of the bounty.

  2. "It was discussed MANY times". - Yes it was, but does this automatically mean that it is not a security vulnerability? Does discussing process reduce the severity of a reported problem?

  3. "Devs have the ability to override their implementation in order to add specific restrictions". Yes devs have this ability. But the fact that a critical severity security vulnerability in your code can be fixed by "devs" does not reduce the severity of the vulnerability in your code, right? Again, your code contains this vulnerability. Your code is in scope. This code: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol What is the point of having a bug bounty if you can just reply "It is not a critical severity vulnerability because third party devs can fix it"?

  4. "Since such restriction is not required by the standard, and some implementations rely on such transfer, we should not disable them by default." - Ok, but it doesn't automatically mean that it is not a security vulnerability. And it doesn't mean that this $1,1B worth of tokens that are stuck in the contract are not permanently frozen. I'm not saying that you must disable transfers. I'm saying that I reported a security vulnerability that fits in your bug bounty program and I want $25,000.

Here is a full description: https://dexaran820.medium.com/known-problems-of-erc20-token-standard-e98887b9532c

Here is a Proof-of-Concept script that calculates how much tokens are currently frozen in the examined contracts: https://dexaran.github.io/erc20_losses/

Dexaran commented 1 year ago

The fact that you will not fix a vulnerability in your code for some reasons does not automatically mean it is not a vulnerability if it fits in your classification criteria and POC was provided.

I'm not saying "fix it". I'm saying "pay me the bounty".

johnpaul-clo commented 1 year ago

@OpenZeppelin Let me know if your Treasury is Empty or you going Bankrupt ? , Follow the standards on your Website and Pay the Hunters

sgitt-vassky commented 1 year ago

Dex, you will give them nightmares at night with your tokens. Although in fact what you write really fits their description of a critical vulnerability on bugbounty page

brandnewx commented 1 year ago

OpenZeppelin bug bounty is about finding a security vulnerability that allows somebody else remotely exploit the contract code, not about the inherent issue with the underlying standards (i.e. ERC-20). If you found a problem with a standard like ERC-20, good for you but you're not the first to notice this. It's been documented for years already.

ERC-20 is far from ideal. In fact, it's one of the foolish designs I've ever encountered in my whole software development career.

Dexaran commented 1 year ago

OpenZeppelin bug bounty is about finding a security vulnerability that allows somebody else remotely exploit the contract code

They have a bug bounty page that describes what their bug bounty is about.

Right now the bug bounty page looks like this:

OpenZeppelin_bug_bounty

I see a phrase "The bug bounty is focused on preventing loss of funds by freezing or theft".

If the ERC20.sol contract is outside of the scope - mark it as "not in scope". But it is not marked.

The fact that the issue is known does not mean it is not a vulnerability. The fact that it was discussed does not mean that this stuck tokens are not frozen.

If it remains in wontfix stage - describe it. Update the bug bounty page and transparently describe "wontfix issues are not subject to bug bounty"

Right now I don't see it and what I have reported fits in critical vulnerabilities by their criteria

Dexaran commented 1 year ago

ERC-20 is far from ideal. In fact, it's one of the foolish designs I've ever encountered in my whole software development career.

Yes, it is one of the foolish designs that caused a lot of people to lose their money.

Dexaran commented 1 year ago

If the bug bounty will be paid I will spend the funds on solving this exact issue of ERC-20 tokens.

My projects are known to adhere to the policy of financial transparency:

I will:

ernestognw commented 1 year ago

Hello @Dexaran, thank you for your contribution to the security of the space. We value bug reports and act upon whenever needed.

In this case, I’m afraid the Bug Bounty program prohibits the following:

Public disclosure of an unpatched vulnerability in an embargoed bounty

Mediation is a process held on Immunefi and we engage with whitehats in a constant, clear and well intentioned conversation. However, this makes it ineligible.

We can focus the conversation on what to do to fix the locked ERC20 issue, instead. We’re planning to support EIP-1363 as an alternative to this issue and we’re open for alternatives. (See #3736)

We’ll come back to this next Monday 👍🏻

Dexaran commented 1 year ago

In this case, I’m afraid the Bug Bounty program prohibits the following:

Public disclosure of an unpatched vulnerability in an embargoed bounty

Well, as @Amxx said "the issue was discussed MANY times" so I don't think that disclosing it 19241th time is anyhow violating the rules if it was disclosed 19240 times before and still not patched.

Mediation is a process held on Immunefi and we engage with whitehats in a constant, clear and well intentioned conversation. However, this makes it ineligible.

You have stopped our conversation on Immunefi - and I have no option to re-open it from my side other than escalate it publicly. Given the nature of the issue that was reported (the fact that it was discussed MANY times in particular) - I don't think it was a wrong move in our particular situation since no one can proactively EXPLOIT an issue and public disclosure of the issue can only PREVENT users from exploiting it and dealing damage to themselves.

However, this makes it ineligible.

Technically I can submit a new bounty via Immunefi and this time I will not be publicly disclosing this same issue 19242th time but I doubt it will make it any more eligible than it is now.

We can focus the conversation on what to do to fix the locked ERC20 issue, instead. We’re planning to support EIP-1363 as an alternative to this issue and we’re open for alternatives.

Great. I will read through it

We’ll come back to this next Monday

See you on Monday, have a nice weekend.

ernestognw commented 1 year ago

I’d suggest you scalating the issue with Immunefi. It’s not the first time mediation is needed after closing a report.

https://immunefisupport.zendesk.com/hc/en-us/articles/10644746170897-Report-Closed-for-Known-Issues

If Immunefi happens to favor your submission we’ll be happy to proceed.

We can focus the conversation on what to do to fix the locked ERC20 issue, instead. We’re planning to support EIP-1363 as an alternative to this issue and we’re open for alternatives.

Great. I will read through it

Amazing, let us know your comments.

Dexaran commented 1 year ago

@ernestognw here is a reply regarding EIP-1363 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3736#issuecomment-1648404823

Honestly I appreciate that there is an effort to solve the problem but I don't think ERC-1363 does it.

frangio commented 1 year ago

You publicly posted about this issue in this repository 2 weeks prior (https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4451). The Immunefi rules very clearly list "Reporting a bug that has already been publicly disclosed" as disqualifying behavior, and the help article linked above says the same. There is no ambiguity about this.

Regardless, this is not an issue of our implementation but an issue in the ERC-20 standard that is well known, both things you said in https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4451. We have considered alternatives to mitigate it and so far we haven't found an option that we feel comfortable implementing. You're free to disagree and prompt us to reconsider, but what you're doing now is not the way to do that.

Dexaran commented 1 year ago

You're free to disagree and prompt us to reconsider, but what you're doing now is not the way to do that.

I would still recommend to update the documentation of your implementation to explicitly state that the ERC20.sol may result in a (well-known) loss of funds for end users especially given the severity of the problem.

As for solution I guess we should move the discussion here: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3736#issuecomment-1648404823

ernestognw commented 1 year ago

As for solution I guess we should move the discussion here: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3736#issuecomment-1648404823

No, that thread is EIP-1363 specific. Discussing EIP-223 should happen in a different thread, and it's not our priority atm. We'll reconsider if there's clear demand from the community and a healthy conversation around it.