search

filecoin-project / specs-actors

DEPRECATED Specification of builtin actors, in the form of executable code.
Other
86 stars 102 forks source link

Confused deputy and caller checking #510

Open Stebalien opened 4 years ago

Stebalien commented 4 years ago

Note: I have no idea if there's any way to actually attack this. However, we should be more careful here.

Currently, when sending messages to actors:

  1. The caller just plugs in the target address + method number, and sends the data.
  2. The receiver unmarshals the data, then sometimes asserts things like "is the caller a system actor".

This works fine as long as the system actor is calling the methods it thinks it's calling (usually). However, this can go very wrong if an attacker can convince the system actor to call the method on the wrong actor (e.g., a system actor).

For example, let's say we implement a "delayed payment" cron job:

  1. Attacker asks cron to pay (invoke method #X on the miner actor type) actor A at time T.
  2. Unless canceled, at time T, cron invokes method X on actor A.

However, the attacker lied. Actor A is actually a different actor. While method X is the pay method for the miner actor, it's a different method on this unexpected actor. Now, this unexpected actor will decode the params as a different type and possibly trust these parameters more than it should because it's being invoked by the trusted cron actor.

The takeaways are:

  1. Ambient authority is dangerous. We should be very careful about relying on caller validation.
  2. Privileged actors need to be very careful about what they call.
  3. Invocations of Send should include the expected receiver type.
ZenGround0 commented 3 years ago

Here's a concrete proposal that gets us most of the way there: Runtime.Send takes in a new argument toCode which the runtime implementation verifies is equal to the code cid of the actor. This forces all callers to limit who they send to.

In reality we will run into scenarios where an exact match isn't good enough and we want to be able to send to many actor (i.e. the Send calls in multisig propose/accept). toCode should actually be a cid set and the runtime implementation should check that toAddr's code cid matches one of the provided actor code cids.

Stebalien commented 3 years ago

Yeah, that would definitely cover it.

I'd love to have some form of address tainting process where all input addresses have a tainted type and all internal addresses get another "verified" type but that might be a larger refactor.