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

Secure DelegateCall #1350

Closed
rmeissner opened this issue Aug 24, 2018 · 6 comments
Closed

Secure DelegateCall #1350

rmeissner opened this issue Aug 24, 2018 · 6 comments
Labels

Comments

@rmeissner
Copy link
Contributor


eip: TDB
title: Secure DelegateCall
author: Richard Meissner (@rmeissner)
category: Core
type: Standards Track
status: Draft
created: 2018-08-24

Specification

Adds a new opcode at 0xf6, which is identical to 0xf4 (delegatecall), but doesn't allow to execute 0xff(SELFDESTRUCT).

Motivation

The use of proxy contract is increasing and with this the use "master copies" with complex code is also increasing. This increases the security risk, if a contract should support delegate calls to any address, to allow the usage of a large selection of library contracts (e.g. a contract that executes multiple transactions at once in the context of a MultiSig). Imposing the requirement that a delegate call is not selfdestructing a contract make it more secure and support audits. With this it can be ensured that a master copy contract can never be suicided and the logic for all the proxies is always available.

Optional

It would also be really helpful to be able to specify a storage range that the delegate call is allowed to access.

@frangio
Copy link
Contributor

frangio commented Aug 24, 2018

I agree something like this would be very valuable. However, the specification needs a lot of work.

If I'm understanding correctly, this would make any attempt to execute selfdestruct throw an exception when reached through a 0xf6 opcode. As a consequence, the caller cannot be selfdestructed. This might be valuable in some scenarios but the stated motivation of the EIP is the opposite: that the master copy (the call target) is not selfdestructed. I don't see how that is guaranteed here. If we were to directly invoke the master copy, instead of through 0xf6, nothing would prevent selfdestruct from executing normally. Have you considered this scenario?

@rmeissner
Copy link
Contributor Author

rmeissner commented Aug 24, 2018

So my motivation was to prevent that a master copy can be suicided.

We have a master copy which allows the use of delegate calls. While the master copy itself has no way of calling selfdestruct it is theoretically possible to call it via a delegate call. Therefore we have to bring the master copy in a state that this is not possible and we should proof that it is not possible to call selfdestruct, since else all the proxies that make use of that master copy are not functional.

It would be nice that this could be ensured on the evm level. So even if somebody somehow finds a way to trigger a delegatecall on that contract it should not be possible to suicide it. (Basically we want to prevent something like openethereum/parity-ethereum#6995)

EDIT: and yes the specification needs work :D just wanted to have some input first, before deep diving into the evm related stuff.

@Arachnid
Copy link
Contributor

This is best worked around at the user level:

  • Don't delegatecall untrusted code (it's a terrible idea anyway)
  • Check that library code is being called from an instance, not directly

@wadealexc
Copy link

So my motivation was to prevent that a master copy can be suicided.

We have a master copy which allows the use of delegate calls. While the master copy itself has no way of calling selfdestruct it is theoretically possible to call it via a delegate call. Therefore we have to bring the master copy in a state that this is not possible and we should proof that it is not possible to call selfdestruct, since else all the proxies that make use of that master copy are not functional.

It would be nice that this could be ensured on the evm level. So even if somebody somehow finds a way to trigger a delegatecall on that contract it should not be possible to suicide it. (Basically we want to prevent something like paritytech/parity-ethereum#6995)

EDIT: and yes the specification needs work :D just wanted to have some input first, before deep diving into the evm related stuff.

A selfdestruct can be mitigated if the caller enforces a revert status from the callee. If the callee does not revert (selfdestruct returns a 'success' status), the caller reverts, protecting state.

The only interaction the caller and callee can have, then, is passing data back via the revert.

@github-actions
Copy link

github-actions bot commented Dec 4, 2021

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Dec 4, 2021
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

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

No branches or pull requests

4 participants