Volver a la wiki

Decisión de seguridad: hardening del confirm Visio (s220) — 3 fixes

Contexto

Auditoría de seguridad (s220) en POST /api/racks/visio/confirm — endpoint que toma la lista de masters (formas convertidas o subidas) y las persiste en la librería de stencils del tenant. Se identificaron 3 vulnerabilidades:

  1. Permiso faltante: confirm no comprobaba permisos; analyze sí. Un usuario autenticado sin permisos admin podía importar stencils (violación de RBAC).
  2. Path-traversal: parámetro image_path con ../ podía resolver fuera de MEDIA_ROOT y mover un fichero arbitrario del servidor a la librería del tenant (LFI → mutación).
  3. XSS persistente: SVG subidos o generados podían contener malware embebido (<script>, handlers, javascript: URLs) sin sanitización.

Decisiones de remediación

Fix 1: Gate de permisos (racks:admin)

Cambio: Añadir require_perm(request, "racks", "admin") en confirm, idéntico a analyze.

Ratificación:

Ubicación: racks/api/library_files.py, línea ~130

Fix 2: Guard anti-path-traversal

Cambio original (s220): Validar image_path con realpath() + prefijo de MEDIA_ROOT antes de mover fichero.

real = os.path.realpath(temp_full_path)
if not real.startswith(os.path.realpath(settings.MEDIA_ROOT) + os.sep):
    continue  # Skip este master, no se persiste

Ratificación (s220):

⚠️ Superado por v1.92.0 (commit c2df243, #478): confinar a MEDIA_ROOT entero resultó insuficiente — un image_path apuntando a signage/<orgB>/… o backups/… (fuera del temp_dir de la sesión pero dentro de MEDIA_ROOT) seguía resolviendo dentro del prefijo válido, y como el propio código usa shutil.move, un master de OTRA organización se movía (y destruía) hacia la librería del tenant atacante. El guard correcto confina al temp_dir de la sesión actual, no a MEDIA_ROOT:

real = os.path.realpath(temp_full_path)
if not is_within_directory(temp_dir, real):
    continue

Usa el helper racks.utils.safe_fs.is_within_directory (véase [[entity—racks—service—safe-fs]]) en vez de comparar prefijos a mano. Mismo patrón aplicado en confirm_restore_backup y racks/api/export/restore_domains.py::_ensure_media_file en el mismo commit.

Ubicación: racks/api/library_files.py, línea ~212 (s220) → confinamiento estrechado a temp_dir en v1.92.0

Fix 3: Defensa en profundidad — sanitización en confirm

Cambio: Aplicar sanitize_svg_file() en el gate final de confirm, antes de mover a librería.

if filename.lower().endswith(".svg"):
    try:
        sanitize_svg_file(temp_full_path)
    except ValueError:
        continue  # Skip este master

Ratificación:

Ubicación: racks/api/library_files.py, línea ~220

Impacto en API contrato

Testing

Nueva suite: tests/api/test_visio_import_v2.py

v1.92.0 añade a tests/racks/test_tanda2_file_confinement.py el caso de un image_path que resuelve dentro de MEDIA_ROOT pero fuera del temp_dir de la sesión (asset de otra org) — antes pasaba el guard de s220, ahora se descarta.

Migración de test existente:

Precedentes

Impacto observacional

Véase también

Subir