Skip to content

Add HTML escaping to sanitize=False computed Html fields #50

@jeremi

Description

@jeremi

Summary

Several computed fields.Html fields use sanitize=False and build HTML via f-strings with unescaped dynamic values. While the practical risk is low (all views are internal/admin-only, and data originates from ORM field values), adding markupsafe.escape() on dynamic content would provide defense-in-depth against stored XSS from malicious internal users or compromised external data intake.

Flagged by Gemini Code Assist on PR #49.

Affected Fields

spp_audit/models/spp_audit_log.py

  • data_html (line ~170): Old/new field values inserted into <td> tags without escaping
  • parent_data_html (line ~213): Same pattern for parent record changes

spp_change_request_v2/models/change_request.py

  • registrant_summary_html (line ~336): reg.name, reg.spp_id, address fields inserted without escaping
  • preview_html (line ~723): Change request field diff values (display_value, display_key) inserted without escaping
  • preview_html_snapshot: Stores snapshot of preview_html, inherits same issue

spp_change_request_v2/wizards/preview_changes_wizard.py

  • preview_html (line ~88): Similar pattern with display_key and display_value

Recommended Fix

Use markupsafe.escape() (or odoo.tools.html_escape) on all dynamic values before inserting into HTML strings:

from markupsafe import escape

# Before
html_parts.append(f"<td>{item}</td>")

# After
html_parts.append(f"<td>{escape(item)}</td>")

Risk Assessment

  • Severity: Low (internal-only views, requires authenticated access to store payload)
  • Impact: An authenticated user could store <script> content in a model field that executes when an admin views audit logs or CR previews
  • Priority: Low — hardening / defense-in-depth

Metadata

Metadata

Labels

enhancementNew feature or request

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions