Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supported versions is not specified #2759

Closed
1 of 4 tasks
fulldecent opened this issue Jul 10, 2021 · 10 comments
Closed
1 of 4 tasks

Supported versions is not specified #2759

fulldecent opened this issue Jul 10, 2021 · 10 comments

Comments

@fulldecent
Copy link
Contributor

fulldecent commented Jul 10, 2021

The OpenZeppelin Contracts project does not specify which versions of the code are supported.

This creates a risk that developers using these contracts will copy paste an old version, develop a contract for years and then publish it because "OZ is safe". This practice is common and I find it in the majority of contract audits I do.

Recommendation

  • Specify in the main README.md exactly which releases are supported and which will continue to receive security updates.
  • Update the [Security Policy] to link to this information https://github.com/OpenZeppelin/openzeppelin-contracts/security/policy.
  • Remove marketing claims that OpenZeppelin Contracts are "audited". These audits do not apply to subsequent (i.e. current) releases and the things that were audited have long been obsoleted.

Follow-on work

@frangio
Copy link
Contributor

frangio commented Jul 16, 2021

Thanks for the suggestions.

Remove marketing claims that OpenZeppelin Contracts are "audited".

Can you point out where we are currently claiming this so we can review it? I believe the readme is quite clear about this.

Reevaluate this statement https://docs.openzeppelin.com/contracts/4.x/releases-stability#solidity-compiler-version if necessary

How do you see this related to the other points?

@fulldecent
Copy link
Contributor Author

fulldecent commented Jul 19, 2021

@frangio Created new issue, and PR, to discuss/fix issues related to the word "audit"

--> #2781
--> #2780

@fulldecent
Copy link
Contributor Author

It is related to the other points in this way:

You and I both know that nobody should use an old version of OpenZeppelin Contracts and that you should never deploy a contract on production using an outdated version of the Solidity compiler. But some people might look at OZC and think "oh... this old version was audited... I should build on that one!"

What is missing is a clear statement from OZC that says "you should not use old versions of the software, only the latest XXX and XXX are supported".

@fulldecent
Copy link
Contributor Author

AH, I misunderstood your question. You want to know how the Solidity Compiler Version is related to mentioning which OZC versions are supported.

This statement is dangerous:

Bug fixes will still be backported to older library releases so that all versions currently in use receive these updates.

This is saying that every "release" of OZC (currently there are 80) is forever maintained for any bug found or error in any (applicable) version of compilers when running this on any network. If true, that is great and remarkable, I would say it closer to that way. But if it's not then the statement should reference which versions are supported and which are not.

@frangio
Copy link
Contributor

frangio commented Jul 23, 2021

You and I both know that nobody should use an old version of OpenZeppelin Contracts

This is not true. Please don't make it sound like we've reached this conclusion in a prior conversation. If there is a serious problem with an old version of OpenZeppelin Contracts it will be disclosed and fixed.

What is less clear is what's considered a serious problem such that it would be backported. We can discuss each particular case in separate issues.

This creates a risk that developers using these contracts will copy paste an old version, develop a contract for years and then publish it because "OZ is safe". This practice is common and I find it in the majority of contract audits I do.

I'm not sure I understand your point here. The problems that I see in this case are careless dependency management and (if I understand correctly) the lack of an audit prior to publishing a contract. That is the source of the risk, and not the fact that they used an older OpenZeppelin version.

@fulldecent
Copy link
Contributor Author

Correct, you and I have not discussed this previously, nor agreed that old versions of OZC should not be used. I went out on a limb and my assertion fails.

If there are arguments that old versions of OZC should not be used I should lay them out here.

The best supporting argument is: OZC depends on Solidity and Solidity specifically recommends that you should use the latest released version and other versions are not supported. (First page of Solidity docs.) This means if a bug is found (maybe a small one) for an old version then it is not guaranteed this will be published. (It might be but that is unspecified behavior.)

Since OZC tells you not to modify OZC at all when using it, that means you should not modify the compiler requirements.

Logically following, any OZC that does not compile under the latest Solidity is not supported upstream. And I argue that OZC should exclude these from any list of supported versions.


If the policy is "all versions are supported" this is weird too. 2.1.3 is a hotfix and 2.1.2 is known bad. So how can we say 2.1.2 is supported? This should be marked bad. Binaries deleted and marked as deprecated in GitHub releases.


PLEASE NOTE everything above is side discussion. I have laid out arguments on some versions that should not be recognized as supported. Reasonable people might think that all, some, or only the latest version are supported, eligible for bug bounty, and known safe.

Since multiple interpretations are possible, here I only wish that we can have an explicit, published policy. And this can possible be in the README, the bug bounty (if/when implemented) and the GitHub security page (if/when implemented).

I hope we can agree a supported policy version is desirable. And as always I am very happy to discuss with you further, propose PRs and support any decisions here.

@frangio
Copy link
Contributor

frangio commented Aug 2, 2021

Solidity specifically recommends that you should use the latest released version and other versions are not supported. (First page of Solidity docs.)

What I can find in their docs is the following:

When deploying contracts, you should use the latest released version of Solidity. This is because breaking changes as well as new features and bug fixes are introduced regularly.

I definitely do not read this as "other versions are not supported". In fact, in the past there have been bugfixes for versions that were not the latest: 0.4.26, 0.5.16, and 0.5.17 were all released after the next minor was already out.


I've updated our documents to clarify the policy. Hopefully it's clearer now. bbd68b7

@fulldecent
Copy link
Contributor Author

Supported versions policy is published, good to close.


If Solidity will more clearly say "do not use old versions", I'll come back here to reconsider supported OZC versions.

@fulldecent
Copy link
Contributor Author

The Solidity project has released a stronger statement to not use old versions. Looping back here to reference:

When deploying contracts, you should use the latest released version of Solidity. Apart from exceptional cases, only the latest version receives security fixes....

https://docs.soliditylang.org/en/latest/

@frangio
Copy link
Contributor

frangio commented Oct 27, 2021

Thanks for pointing that out. I've added a mention of the Solidity policy in our own security policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants