From 31c616abefaa7317574add9183cfd8e0671cfd88 Mon Sep 17 00:00:00 2001 From: Proudlock <270015376@geaerospace.com> Date: Wed, 9 Sep 2026 09:10:43 -0400 Subject: [PATCH] Fix critical XSS vulnerabilities in operator report email alerts SECURITY FIXES: - Escape all user-controlled values (hostname, reason, details) to prevent XSS - Fix endpoint name from 'assets.detail' to 'assets.get_asset' - Fix fallback URL to be absolute instead of relative path - Remove render_email() to avoid subject duplication in email body - Check recipients exist before expensive HTML generation - Build email template directly with proper escaping throughout --- plugins/endpoints/api/routes.py | 55 +++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 533ba1a4..3a0e1a57 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -32,7 +32,8 @@ from ..models import ( ) from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK from ..eventcatalog import CRITICAL_EVENT_IDS, event_severity, event_title -from shopdb.utils.mailer import send_alert, render_email +from shopdb.utils.mailer import send_alert +from markupsafe import escape endpoints_bp = Blueprint('endpoints', __name__) @@ -2938,18 +2939,30 @@ def _purge_expired_operator_reports(): def _send_operator_report_alert(report, asset_id, hostname): """Send email alert for new operator issue report to default alert recipients.""" - from flask import url_for + from flask import url_for, request + from shopdb.utils.mailer import get_smtp_config, _normalize_recipients + + # Check if we have recipients before doing expensive work + config = get_smtp_config() + recipients = _normalize_recipients(config['alert_recipients']) + if not recipients: + current_app.logger.debug('Skipping operator report alert: no alert recipients configured') + return asset = Asset.query.get(asset_id) if not asset: current_app.logger.warning('Cannot send operator report alert: asset %d not found', asset_id) return + # Generate absolute URLs try: - device_url = url_for('assets.detail', asset_id=asset_id, _external=True) + device_url = url_for('assets.get_asset', asset_id=asset_id, _external=True) except Exception as e: current_app.logger.error('Failed to generate device URL: %s', e) - device_url = f'/shopdb/assets/{asset_id}' + # Build absolute URL manually using request context + scheme = request.scheme if request else 'https' + host = request.host if request else 'localhost' + device_url = f'{scheme}://{host}/shopdb/assets/{asset_id}' reported_at = report.createdat.strftime('%Y-%m-%d %H:%M:%S UTC') if report.createdat else 'Unknown' @@ -2959,32 +2972,42 @@ def _send_operator_report_alert(report, asset_id, hostname): screenshot_url = url_for('endpoints.serve_operator_report_image', operatorreportid=report.operatorreportid, _external=True) - screenshot_info = f'

Screenshot: View Screenshot

' + screenshot_info = f'

Screenshot: View Screenshot

' except Exception as e: current_app.logger.warning('Failed to generate screenshot URL: %s', e) - details_html = f'

{report.details or "No additional details provided."}

' + # HTML-escape all user-controlled values to prevent XSS + safe_hostname = escape(hostname) + safe_computername = escape(asset.computername or hostname) + safe_assettag = escape(asset.assettag or 'N/A') + safe_reason = escape(report.reason) + safe_details = escape(report.details or 'No additional details provided.') - body_html = f''' + # Build HTML body (don't use render_email to avoid subject duplication) + html_body = f''' +
+

Operator Issue Report

New Issue Report from Shop Floor

-

Device: {asset.computername or hostname} ({hostname})

-

Asset Tag: {asset.assettag or 'N/A'}

-

Reason: {report.reason}

-

Reported: {reported_at}

+

Device: {safe_computername} ({safe_hostname})

+

Asset Tag: {safe_assettag}

+

Reason: {safe_reason}

+

Reported: {escape(reported_at)}

Details:

-{details_html} +

{safe_details}

{screenshot_info} -

View Device in ShopDB

+

View Device in ShopDB

+
+

Sent by ShopDB.

+
''' - subject = f'Operator Issue Report: {asset.computername or hostname}' - html, text = render_email(subject, body_html) + subject = f'Operator Issue Report: {safe_computername}' current_app.logger.info('Sending operator report alert for %s (report %d)', hostname, report.operatorreportid) - send_alert(subject, html, text=text) + send_alert(subject, html_body) @endpoints_bp.route('/operatorreports', methods=['POST'])