Update Software Engineering Upgradeability section #244
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Update Software Engineering upgradeability section Registry * Update sample to Solidity 0.5.0 * Add address validation "changeBackend" function Relay * Add risks and recommendations regarding the use of delegatecall * Addresses security issues regarding proxies * Add LogicContract sample * Update sample to Solidity 0.5.0 * Add address validation "constructor" and "payable" function
|
Thanks @fodisi! One of us will review this when we get a chance next week. |
shayanb
reviewed
Jan 7, 2020
| - [How a contract's constructor can affect upgradability](https://blog.openzeppelin.com/towards-frictionless-upgradeability/) | ||
| - How the ABI specifies [function selectors](https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html?highlight=signature#function-selector) and how [function-name collision](https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357) can be used to exploit a contract that uses `delegatecall` | ||
| - How `delegatecall` to a non-existent contract will return true even if the called contract does no exist. For more details see [Breaking the proxy pattern](https://blog.trailofbits.com/2018/09/05/contract-upgrade-anti-patterns/) and Solidity docs on [Error handling](https://solidity.readthedocs.io/en/latest/control-structures.html#error-handling-assert-require-revert-and-exceptions). | ||
|
|
There was a problem hiding this comment.
I suggest adding this blog post to the resources as well: https://diligence.consensys.net/blog/2019/01/upgradeability-is-a-bug/
There was a problem hiding this comment.
@shayanb
Thank you for your review. I just committed the changes to add a reference to the blog post provided.
shayanb
approved these changes
Jan 7, 2020
|
@shayanb @maurelian Thank you! |
|
@fodisi thanks for the PR and also reminding us to merge it :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Registry
Relay