Skip to content

Conversation

@cameel
Copy link

@cameel cameel commented Apr 28, 2022

The reason behind the PR has already been explained well by @chriseth in a similar PR to OpenZeppelin (OpenZeppelin/openzeppelin-contracts#3293) so I'll copy that description here:

First, I would like to apologize. Somehow the issue ethereum/solidity#10900 got lost in our backlog. We are currently fixing it and discovered one instance of this in openzeppelin. This PR fixes it.

More details:

The function hashProposal in the interface is declared with calldata parameters while its implementation in Governor uses memory. This leads to invalid code being generated whenever someone calls the hashProposal function internally on the interface instead of the implementation. This can only happen if you have an (abstract) contract that inherits from the interface, but not from the implementation, but is still part of an inheritance hierarchy that also has the implementation.

Simplified example:

abstract contract I {
        function f(uint[] calldata x)  virtual internal;
}
contract C is I {
        uint public data;
        // The override checker in the compiler does not complain
        // here - this is the bug in the compiler.
        function f(uint[] memory x)  override internal {
                data = x[0];
        }
}
abstract contract D is I {
        function g(uint[] calldata x)  external {
                // Since D only "knows" `I`, the signature of `f` uses calldata,
                // while the virtual lookup ends up with `C.f`, which uses memory.
                // This results in the calldata pointer `x` being passed and interpreted
                // as a memory pointer.
                f(x);
        }
}
contract X is C, D { }

In case of Brink, the function in question is isValidSignature from ISignatureValidator.

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

Successfully merging this pull request may close these issues.

1 participant