okTurtles / group-income

A decentralized and private (end-to-end encrypted) financial safety net for you and your friends.
https://groupincome.org
GNU Affero General Public License v3.0
331 stars 44 forks source link

Sandboxing selectors called from contracts #2364

Open corrideat opened 1 month ago

corrideat commented 1 month ago

Sandboxing selectors

This is a proposal for how to handle contracts calling arbitrary selectors. Currently, contracts are exposed to the contractSBP wrapper that uses an allow list. While it is useful and desirable for contacts to have this ability, it means that sandboxing cannot be done meaningfully.

For example, the chelonia/contract/state selector, while useful, could be used to read data that is not in the scope of the contract making the call. Similarly, the chelonia/queueInvocation selector, while also useful, could be (mis)used to introduce (un)intended deadlocks.

Selectors that call other selectors

The chelonia/queueInvocation selector is an interesting case because it can be used both to queue a function and to queue a selector. For example, sbp('chelonia/queueInvocation', QUEUE_ID, () => { /* ... */ }) and sbp('chelonia/queueInvocation', QUEUE_ID, ['some/selector']) are both valid invocations.

The first case is not particularly special from a security standpoint because a function definition will have the same restrictions as the context in which it is defined. However, the second case (calling a selector) requires careful consideration because it could be used to escape the restrictions placed on the contract. In other words, there is a real possibility of the invocation being made with the permissions of the root context instead of the more restricted contract context.

Solution

This proposal recognises the usefulness of being able to call external methods as well as the need to avoid refactoring existing code where possible.

The current contractSBP definition could be changed from a general wrapper (as it is now) to a per-selector wrapper that enforces containment rules.

For example, calling chelonia/contract/state from within a contract would call a special wrapper that understands the semantics of that selector and verifies that the contract ID provided is for a contract of that type. Similarly, the names of queues available through chelonia/queueInvocation could be restricted to follow certain rules. If validation passes, the wrapper would call the 'real' selector and return the result (possibly making certain transformations).

Example:

    const allowedSels = {
        'chelonia/contract/state': (_, contractName, manifestHash, ...args) => {
            const contractID = args[1]
            const cheloniaState = sbp('chelonia/rootState')
            if (cheloniaState.contracts[contractID].type !== contractName) {
                throw new Error('Type does not match')
            }
            return sbp('chelonia/contract/state', ...args)
        }
    }
    const contractSBP = (selector: string, ...args) => {
      const domain = domainFromSelector(selector)
      if (selector.startsWith(contractName + '/')) {
        selector = `${manifestHash}/${selector}`
      }
      const wrapper = allowedSels[selector] || allowedDoms[domain]
      if (wrapper) {
        return wrapper(selector, contractName, manifestHash, ...args)
      } else {
        console.error('[chelonia] selector not on allowlist', { selector, allowedSels, allowedDoms })
        throw new Error(`[chelonia] selector not on allowlist: '${selector}'`)
      }
    }

Selectors that call other selectors

To solve the issue of escaping these restrictions by using a selector that calls another selector (e.g., sbp('chelonia/queueInvocation', QUEUE_ID, ['chelonia/queueInvocation', Math.random(), ['some/privileged/action']])), there are two possible approaches:

  1. The safest and simplest option is to forbid these invocations in contracts. However, that goes against the spirit of being able to use selectors.
  2. The second alternative is for the wrapper to re-implement these calls by wrapping it using contractSBP. This option requires being wary of all potential ways that an privileged call could be made and for the wrapper to also have access to contractSBP.
    const allowedSels = {
        'chelonia/queueInvocation': (_, contractName, manifestHash, contractSBP, ...args) => {
            const queueName = args[0]
            let invocation = args[1]
            validateQueueName(queueName)
            // Wrap selector calls
            if (Array.isArray(invocation)) invocation = () => contractSBP(...invocation)
            return sbp(''chelonia/queueInvocation', queueName, invocation)
        }
    }

Pros

Simple and works with existing code mostly unmodified.

Cons

Requires writing custom wrappers for each selector that is allowed and keeping up with API changes.

Alternatives

Alternatively, Chelonia state could be partitioned to have a separate partition per contract type (for example, similarly to how modern browsers handle third-party cookies). This would require more significant changes to Chelonia, and it would still be insufficient to constrain selectors that aren't directly Chelonia-related, such as, for example, gi.actions/group/join or gi.notifications/emit.

taoeffect commented 1 month ago

chelonia/queueInvocation selector, while also useful, could be (mis)used to introduce (un)intended deadlocks.

Not just deadlocks, it could be used to call non-whitelisted selectors too and that also needs to be addressed.

corrideat commented 1 month ago

it could be used to call non-whitelisted selectors too and that also needs to be addressed.

Fair point. I had forgotten about that. That's not a concern when using a function callback, but when using selectors, it'd probably require passing the contractSBP function to the handler.