Skip to content

security(powerpoint): recursive processing without depth limits #1015

@WilliamBerryiii

Description

@WilliamBerryiii

Summary

Theme reference resolution and group element building in the PowerPoint skill codebase use recursive processing without depth limits. Crafted inputs with deeply nested structures can trigger stack exhaustion (Python's default recursion limit is 1000 frames).

Location

  • File: .github/skills/experimental/powerpoint/scripts/extract_content.py — theme reference resolution chains
  • File: .github/skills/experimental/powerpoint/scripts/build_deck.py — group element building with nested groups
  • Potentially other modules: Any function processing nested YAML/PPTX structures recursively

Risk Assessment

  • Severity: MODERATE
  • Attack Vector: A crafted PPTX file with deeply nested group elements or circular theme references, or a YAML configuration with deeply nested structures
  • Impact: RecursionError causing process crash (denial of service)
  • CVSS Category: CWE-674 (Uncontrolled Recursion)

Current Behavior

Recursive functions process nested structures without:

  • Maximum depth parameters
  • Depth counters or guards
  • Cycle detection for reference chains
  • Explicit sys.setrecursionlimit() protection

Expected Behavior

Recursive processing should include:

  1. Depth limit parameter: Each recursive function accepts a max_depth parameter with a sensible default (e.g., 20 for PPTX group nesting, 10 for theme reference chains)
  2. Depth counter: Each recursive call increments a counter and raises a clear error when the limit is exceeded
  3. Cycle detection: Reference resolution tracks visited nodes to detect circular references
  4. Clear error messages: Depth limit violations produce descriptive errors indicating the nesting path

RPI Framework

task-researcher

  • Identify all recursive functions in the codebase
  • Determine realistic maximum nesting depths for PPTX group elements and theme references
  • Check if python-pptx itself imposes any nesting limits
  • Assess whether YAML safe_load already limits nesting depth (it does not by default)

task-planner

  • Define appropriate depth limits for each recursive context
  • Decide between depth parameter approach vs decorator-based guard
  • Plan consistent error handling for depth limit violations

task-implementor

  • Add max_depth parameter to recursive functions
  • Implement depth counting with clear ValueError on limit exceeded
  • Add cycle detection for reference resolution (visited set)
  • Add tests with deeply nested inputs verifying depth limits are enforced
  • Add tests with circular references verifying cycle detection

Acceptance Criteria

  • All recursive functions in the PowerPoint skill have explicit depth limits
  • Depth limit violations raise descriptive errors (not RecursionError)
  • Reference resolution includes cycle detection to prevent infinite loops
  • Depth limits are configurable with sensible defaults
  • Tests verify that inputs exceeding depth limits are rejected with clear error messages
  • Tests verify that circular references are detected and reported

Metadata

Metadata

Labels

bugSomething isn't workingsecuritySecurity-related changes or concernsskillsCopilot skill packages (SKILL.md)

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions