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
This commit is contained in:
@@ -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'<p><strong>Screenshot:</strong> <a href="{screenshot_url}">View Screenshot</a></p>'
|
||||
screenshot_info = f'<p><strong>Screenshot:</strong> <a href="{escape(screenshot_url)}">View Screenshot</a></p>'
|
||||
except Exception as e:
|
||||
current_app.logger.warning('Failed to generate screenshot URL: %s', e)
|
||||
|
||||
details_html = f'<p style="white-space: pre-wrap;">{report.details or "No additional details provided."}</p>'
|
||||
# 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'''
|
||||
<div style="font-family:Arial,Helvetica,sans-serif;max-width:640px;margin:0 auto;color:#222;">
|
||||
<h2 style="color:#1a1a1a;margin:0 0 12px;">Operator Issue Report</h2>
|
||||
<div style="padding: 16px; background: #f9f9f9; border-left: 4px solid #d93f3f; margin-bottom: 16px;">
|
||||
<p style="margin: 0 0 8px; font-weight: bold; color: #d93f3f;">New Issue Report from Shop Floor</p>
|
||||
<p style="margin: 0;"><strong>Device:</strong> <a href="{device_url}">{asset.computername or hostname}</a> ({hostname})</p>
|
||||
<p style="margin: 0;"><strong>Asset Tag:</strong> {asset.assettag or 'N/A'}</p>
|
||||
<p style="margin: 0;"><strong>Reason:</strong> {report.reason}</p>
|
||||
<p style="margin: 0;"><strong>Reported:</strong> {reported_at}</p>
|
||||
<p style="margin: 0;"><strong>Device:</strong> <a href="{escape(device_url)}">{safe_computername}</a> ({safe_hostname})</p>
|
||||
<p style="margin: 0;"><strong>Asset Tag:</strong> {safe_assettag}</p>
|
||||
<p style="margin: 0;"><strong>Reason:</strong> {safe_reason}</p>
|
||||
<p style="margin: 0;"><strong>Reported:</strong> {escape(reported_at)}</p>
|
||||
</div>
|
||||
<p><strong>Details:</strong></p>
|
||||
{details_html}
|
||||
<p style="white-space: pre-wrap;">{safe_details}</p>
|
||||
{screenshot_info}
|
||||
<p><a href="{device_url}" style="display: inline-block; padding: 10px 16px; background: #1a73e8; color: white; text-decoration: none; border-radius: 4px;">View Device in ShopDB</a></p>
|
||||
<p><a href="{escape(device_url)}" style="display: inline-block; padding: 10px 16px; background: #1a73e8; color: white; text-decoration: none; border-radius: 4px;">View Device in ShopDB</a></p>
|
||||
<hr style="border:none;border-top:1px solid #ddd;margin:24px 0 12px;">
|
||||
<p style="font-size:12px;color:#888;margin:0;">Sent by ShopDB.</p>
|
||||
</div>
|
||||
'''
|
||||
|
||||
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'])
|
||||
|
||||
Reference in New Issue
Block a user