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

Caller-side restrictions on method capabilities #725

Closed
anorth opened this issue Jun 27, 2022 · 4 comments
Closed

Caller-side restrictions on method capabilities #725

anorth opened this issue Jun 27, 2022 · 4 comments

Comments

@anorth
Copy link
Member

anorth commented Jun 27, 2022

An idea via @jsuresh (working with me on token API design).

Re-entrancy bugs are rather common in existing blockchain systems (see The DAO hack and many others), and Filecoin isn't immune. They can sometimes be exploited/attacked by a malicious receiver.

Some cases could be prevented if a caller could restrict the capabilities of a callee method, such as to disallow the use of send(). Then a high-value contract could call hooks/receivers/getters with no risk of a malicious receiver calling back to exploit an unsaved state. Similarly a method could be forced to be read-only by restricting writes. I imagine these restrictions might be switched with a flag to the send() syscall.

An alternative to runtime enforcement would be compile-time, e.g. a readonly attribute on a method, enforced by the VM. But note that this would have to be done in a way that callers can rely on. E.g. if a standard API specified a GetThing() readonly method, it must be impossible for an actor to implement GetThing (not readonly) and satisfy the API. I can't think how to do that at the moment, given internal dispatch.

I would note that preventing send() would restrict some otherwise positive design patterns like delegation, and the general preference for many small interacting contracts over monoliths.

These runtime restrictions could share mechanism with similar restrictions for account abstraction.

@Stebalien
Copy link
Member

Two alternatives:

  1. Transactional memory: the actor SDK could check to see if the state-root has changed since last reading/writing it, and fail (unless the method was marked as "safe" for re-entrency). Unfortunately, this doesn't:
    1. Prevent reads while the actor is in some invalid state.
    2. Prevent the ABA problem (unless we introduce a state version, which honestly wouldn't be a terrible idea).
  2. Reverts: Filecoin already has state-tree transactions, and we could expose those to users. That is:
    1. Open a filecoin-level transaction.
    2. Perform 1 or more sends to read information.
    3. Abort the transaction.

@raulk raulk added the MIGRATED label Aug 18, 2022
@raulk raulk transferred this issue from filecoin-project/fvm-specs Aug 18, 2022
@Stebalien
Copy link
Member

See filecoin-project/FIPs#487

@Stebalien
Copy link
Member

Implemented.

@Stebalien
Copy link
Member

In #1150

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

No branches or pull requests

3 participants