Local delegation validation silently skips the `allowedAction` check when `parentCapability` is a string

Abierto
  • ¿Algo no está bien?
#91 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
48/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
javascript
Área
authorization, security

Línea de trabajo

Empieza por lib/CapabilityDelegation.js:97, 122 y 158; después, lee utils.hasValidAllowedAction y las comprobaciones de root-capability alrededor de lib/utils.js:528 y 567. Ejecuta la reproducción usando un parentCapability de tipo string y compárala con la forma de objeto. Se considera terminado cuando el proyecto tenga un comportamiento acordado para los roots de tipo string, la validación local coincida con la verificación y la regresión esté cubierta.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Summary

CapabilityDelegation accepts parentCapability as either a full capability object or "a string expressing the ID of a root capability" (lib/CapabilityDelegation.js:97). When the string form is used, the local allowedAction restrictiveness check in update() reads the property off that string, gets undefined, and passes vacuously. A delegator can therefore sign a capability that is less restrictive than its root, with no error, and only discover the problem when a verifier rejects it.

Mechanism

lib/CapabilityDelegation.js:158:

const {allowedAction: parentAllowedAction} = parentCapability;

For a string this is undefined, so utils.hasValidAllowedAction short-circuits on if(!parentAllowedAction) return true and the check never runs.

A root capability is precisely the case where this matters. checkCapability permits allowedAction on root capabilities (the check at lib/utils.js:567 sits outside the root/delegated branches), and #90 added enforcement of a root's allowedAction at verification time. So the string form is the one path where a delegator can set allowedAction against a root that constrains it and get no local feedback.

Note that the neighboring expires check is not affected: checkCapability forbids expires on root capabilities (lib/utils.js:528), so parentExpires is legitimately undefined for a root whether it is passed by ID or as an object.

Reproduction

Root gamma declares allowedAction: 'write'. Delegating from its ID string with allowedAction: ['read', 'write'] signs cleanly, then fails verification:

const zcap = await _delegate({
  newCapability: {
    '@context': ZCAP_CONTEXT_URL,
    id: uuid(),
    controller: bob.id(),
    parentCapability: capabilities.root.gamma.id,
    invocationTarget: capabilities.root.gamma.invocationTarget,
    allowedAction: ['read', 'write'],
    expires: EXPIRES_3000_DATE
  },
  // string form of the root zcap
  parentCapability: capabilities.root.gamma.id,
  delegator: alice
});
// no error thrown

const r = await _verifyDelegation({
  delegation: zcap,
  expectedRootCapability: capabilities.root.gamma.id
});
// r.verified === false
// 'The "allowedAction" in a delegated capability must not be less restrictive
//  than its parent.'

Passing capabilities.root.gamma (the object) instead throws at signing time, as intended.

Impact

This is a correctness and developer-experience problem rather than a security hole: verification catches the bad capability, so nothing invalid is accepted. But it lets a delegator mint capabilities that are dead on arrival, and the failure surfaces far from its cause. That matters more now that #90 makes a root's allowedAction load-bearing at verification time - a case that previously failed silently and harmlessly now produces unusable capabilities.

Options

  1. Dereference the root capability during local validation and check against the real document. documentLoader is available to update() (it is currently destructured away - lib/CapabilityDelegation.js:122 takes only {document}). This is the correct fix, but it introduces loader I/O into proof creation where there is none today, and makes delegation fail when the root is unreachable. That tradeoff deserves a deliberate decision.

  2. Reject the string form outright and require the full parent capability object. Simplest and safest, but a breaking API change.

  3. Document the limitation and leave local validation as best-effort.

Lenguaje dominante
JavaScript
Estrellas
29
Forks
3
Sin PR fusionados en 30 d

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de digitalbazaar/zcap

Todos los issues de digitalbazaar/zcap

Issues similares

Más issues de JavaScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.