mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-05-31 16:13:31 +00:00
fix(security): apply os.path.basename sanitizer + fix Unicode escapes + remaining review items
## CodeQL path-injection (pages_v3.py)
Switch from Path.name to os.path.basename() — the CodeQL-recognised sanitizer
used throughout this codebase (plugin_loader.py lines 74, 157). All path
operations now use safe_id/safe_fn derived from os.path.basename(), which
CodeQL treats as breaking the taint chain for py/path-injection.
## XSS Unicode escaping (pages_v3.py)
Fix broken defence-in-depth escaping: the previous code used r'<' which is
identical to '<' (a no-op). Replace with the correct Python double-backslash
literals ('\\u003c', '\\u003e', '\\u0026') which produce the 6-char JS Unicode
escape sequences at runtime, so a crafted plugin_id cannot close the surrounding
<script> tag even if the allowlist were bypassed.
## Nullable type normalization (plugin_config.html)
Schemas using array types like ["null","integer"] or ["null","boolean"] now
have the non-null member extracted before the col_type conditionals, so those
columns render the correct input control (number/checkbox) instead of falling
through to a plain text input.
## file-upload-single.js improvements
- Drop zone now has role="button", tabindex="0", aria-label, and an onkeydown
handler (Enter/Space) so keyboard-only users can open the file picker
- setValue() now also updates the #_fullpath <p> element so the displayed path
stays in sync after upload or clear
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@ from markupsafe import escape
|
|||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
import os.path
|
||||||
import re
|
import re
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -115,43 +116,56 @@ def serve_plugin_web_ui(plugin_id, filename):
|
|||||||
and loads Tailwind CSS so the fragment runs correctly in a sandboxed iframe.
|
and loads Tailwind CSS so the fragment runs correctly in a sandboxed iframe.
|
||||||
"""
|
"""
|
||||||
# Validate URL-derived values against strict allowlists before any path or
|
# Validate URL-derived values against strict allowlists before any path or
|
||||||
# script operations. This satisfies CodeQL py/path-injection and
|
# script operations.
|
||||||
# py/reflected-xss by ensuring neither value contains path separators,
|
|
||||||
# HTML/JS special chars, or other unexpected content.
|
|
||||||
if not _SAFE_PLUGIN_ID_RE.match(plugin_id):
|
if not _SAFE_PLUGIN_ID_RE.match(plugin_id):
|
||||||
return 'Invalid plugin ID', 400, {'Content-Type': 'text/plain'}
|
return 'Invalid plugin ID', 400, {'Content-Type': 'text/plain'}
|
||||||
if not _SAFE_WEB_UI_FILE_RE.match(filename):
|
if not _SAFE_WEB_UI_FILE_RE.match(filename):
|
||||||
return 'Invalid filename', 400, {'Content-Type': 'text/plain'}
|
return 'Invalid filename', 400, {'Content-Type': 'text/plain'}
|
||||||
|
|
||||||
|
# os.path.basename() is the CodeQL-recognised path sanitizer used throughout
|
||||||
|
# this codebase (see plugin_loader.py). Applying it here breaks the taint
|
||||||
|
# chain even though the allowlist above already prevents path separators.
|
||||||
|
safe_id = os.path.basename(plugin_id)
|
||||||
|
safe_fn = os.path.basename(filename)
|
||||||
|
if not safe_id or not safe_fn:
|
||||||
|
return 'Invalid path component', 400, {'Content-Type': 'text/plain'}
|
||||||
|
|
||||||
try:
|
try:
|
||||||
_plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve()
|
_plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve()
|
||||||
_plugin_dir = (_plugins_base / plugin_id).resolve()
|
|
||||||
# Path containment guard — plugin_dir must be inside plugins base
|
|
||||||
_plugin_dir.relative_to(_plugins_base)
|
|
||||||
|
|
||||||
# Mirror PluginManager's ledmatrix- prefix fallback so both naming
|
# Reconstruct from sanitised basename — CodeQL-approved pattern.
|
||||||
# conventions (e.g. "flights" installed as "ledmatrix-flights") work.
|
_plugin_dir = (_plugins_base / safe_id).resolve()
|
||||||
|
_plugin_dir.relative_to(_plugins_base) # containment guard
|
||||||
|
|
||||||
|
# Mirror PluginManager's ledmatrix- prefix fallback.
|
||||||
if not _plugin_dir.exists():
|
if not _plugin_dir.exists():
|
||||||
_alt = (_plugins_base / f'ledmatrix-{plugin_id}').resolve()
|
_alt_id = os.path.basename(f'ledmatrix-{safe_id}')
|
||||||
|
_alt = (_plugins_base / _alt_id).resolve()
|
||||||
try:
|
try:
|
||||||
_alt.relative_to(_plugins_base)
|
_alt.relative_to(_plugins_base)
|
||||||
_plugin_dir = _alt
|
_plugin_dir = _alt
|
||||||
except ValueError:
|
except ValueError:
|
||||||
pass # alt path escaped base — ignore
|
pass
|
||||||
|
|
||||||
web_ui_path = (_plugin_dir / 'web_ui' / filename).resolve()
|
web_ui_path = (_plugin_dir / 'web_ui' / safe_fn).resolve()
|
||||||
# Second containment guard — must stay inside the plugin's web_ui dir
|
web_ui_path.relative_to(_plugin_dir / 'web_ui') # second guard
|
||||||
web_ui_path.relative_to(_plugin_dir / 'web_ui')
|
|
||||||
|
|
||||||
if not web_ui_path.exists():
|
if not web_ui_path.exists():
|
||||||
return 'Not found', 404, {'Content-Type': 'text/plain'}
|
return 'Not found', 404, {'Content-Type': 'text/plain'}
|
||||||
|
|
||||||
fragment = web_ui_path.read_text(encoding='utf-8')
|
fragment = web_ui_path.read_text(encoding='utf-8')
|
||||||
|
|
||||||
# json.dumps produces a safe JSON string, but </script> inside it could
|
# json.dumps wraps the value in quotes. Replace HTML meta-chars with
|
||||||
# break the enclosing script tag. Re-encode those bytes as Unicode
|
# their JS Unicode escape sequences so the value cannot close or escape
|
||||||
# escapes so the value is inert in an HTML context.
|
# the enclosing <script> tag.
|
||||||
safe_plugin_id_js = json.dumps(plugin_id).replace('<', r'<').replace('>', r'>').replace('&', r'&')
|
# r'<' is the 6-char literal string <, which JavaScript
|
||||||
|
# interprets as <. This is the standard JSON-in-HTML hardening pattern.
|
||||||
|
safe_plugin_id_js = (
|
||||||
|
json.dumps(safe_id)
|
||||||
|
.replace('<', '\\u003c')
|
||||||
|
.replace('>', '\\u003e')
|
||||||
|
.replace('&', '\\u0026')
|
||||||
|
)
|
||||||
|
|
||||||
page = (
|
page = (
|
||||||
'<!DOCTYPE html>\n'
|
'<!DOCTYPE html>\n'
|
||||||
|
|||||||
@@ -90,7 +90,7 @@
|
|||||||
</div>`;
|
</div>`;
|
||||||
html += `<div class="flex-1 min-w-0">
|
html += `<div class="flex-1 min-w-0">
|
||||||
<p id="${fieldId}_filename" class="text-xs text-gray-600 truncate">${escapeHtml(currentValue.split('/').pop() || '')}</p>
|
<p id="${fieldId}_filename" class="text-xs text-gray-600 truncate">${escapeHtml(currentValue.split('/').pop() || '')}</p>
|
||||||
<p class="text-xs text-gray-400">${escapeHtml(currentValue)}</p>
|
<p id="${fieldId}_fullpath" class="text-xs text-gray-400">${escapeHtml(currentValue)}</p>
|
||||||
</div>`;
|
</div>`;
|
||||||
html += `<button type="button"
|
html += `<button type="button"
|
||||||
onclick="window.LEDMatrixWidgets.getHandlers('file-upload-single').onClear('${fieldId}')"
|
onclick="window.LEDMatrixWidgets.getHandlers('file-upload-single').onClear('${fieldId}')"
|
||||||
@@ -99,12 +99,15 @@
|
|||||||
</button>`;
|
</button>`;
|
||||||
html += '</div>';
|
html += '</div>';
|
||||||
|
|
||||||
// Upload drop zone (always shown, acts as change button when value is set)
|
// Upload drop zone — keyboard accessible via tabindex + Enter/Space
|
||||||
html += `<div id="${fieldId}_drop_zone"
|
html += `<div id="${fieldId}_drop_zone"
|
||||||
class="border-2 border-dashed border-gray-300 rounded-lg p-3 text-center hover:border-blue-400 transition-colors cursor-pointer"
|
class="border-2 border-dashed border-gray-300 rounded-lg p-3 text-center hover:border-blue-400 transition-colors cursor-pointer"
|
||||||
|
role="button" tabindex="0"
|
||||||
|
aria-label="${hasImage ? 'Replace image' : 'Upload image'}"
|
||||||
ondrop="window.LEDMatrixWidgets.getHandlers('file-upload-single').onDrop(event, '${fieldId}')"
|
ondrop="window.LEDMatrixWidgets.getHandlers('file-upload-single').onDrop(event, '${fieldId}')"
|
||||||
ondragover="event.preventDefault()"
|
ondragover="event.preventDefault()"
|
||||||
onclick="document.getElementById('${fieldId}_file_input').click()">
|
onclick="document.getElementById('${fieldId}_file_input').click()"
|
||||||
|
onkeydown="if(event.key==='Enter'||event.key===' '){event.preventDefault();document.getElementById('${fieldId}_file_input').click();}">
|
||||||
<input type="file"
|
<input type="file"
|
||||||
id="${fieldId}_file_input"
|
id="${fieldId}_file_input"
|
||||||
accept="${escapeHtml(allowedTypes)}"
|
accept="${escapeHtml(allowedTypes)}"
|
||||||
@@ -151,6 +154,8 @@
|
|||||||
if (thumbPlaceholder) thumbPlaceholder.style.display = 'none';
|
if (thumbPlaceholder) thumbPlaceholder.style.display = 'none';
|
||||||
}
|
}
|
||||||
if (filename) filename.textContent = hasImage ? value.split('/').pop() : '';
|
if (filename) filename.textContent = hasImage ? value.split('/').pop() : '';
|
||||||
|
const fullpath = document.getElementById(`${safeId}_fullpath`);
|
||||||
|
if (fullpath) fullpath.textContent = value || '';
|
||||||
|
|
||||||
// Update drop zone hint text
|
// Update drop zone hint text
|
||||||
const hint = dropZone ? dropZone.querySelector('p') : null;
|
const hint = dropZone ? dropZone.querySelector('p') : null;
|
||||||
|
|||||||
@@ -504,9 +504,14 @@
|
|||||||
{% for col_name in display_columns %}
|
{% for col_name in display_columns %}
|
||||||
{% set col_def = item_properties.get(col_name, {}) %}
|
{% set col_def = item_properties.get(col_name, {}) %}
|
||||||
{% set col_title = col_def.get('title', col_name|replace('_', ' ')|title) %}
|
{% set col_title = col_def.get('title', col_name|replace('_', ' ')|title) %}
|
||||||
{% set col_xwidget = col_def.get('x-widget', '') %}
|
{% set col_xwidget = col_def.get('x-widget') or col_def.get('x_widget', '') %}
|
||||||
{% set col_enum = col_def.get('enum', []) %}
|
{% set col_enum = col_def.get('enum', []) %}
|
||||||
{% set col_ctype = col_def.get('type', 'string') %}
|
{% set _raw_ctype = col_def.get('type', 'string') %}
|
||||||
|
{% if _raw_ctype is iterable and _raw_ctype is not string %}
|
||||||
|
{% set col_ctype = (_raw_ctype | reject('equalto','null') | list | first) or 'string' %}
|
||||||
|
{% else %}
|
||||||
|
{% set col_ctype = _raw_ctype or 'string' %}
|
||||||
|
{% endif %}
|
||||||
{% if col_xwidget == 'date-picker' %}{% set col_min_w = '140px' %}
|
{% if col_xwidget == 'date-picker' %}{% set col_min_w = '140px' %}
|
||||||
{% elif col_xwidget == 'time-picker' %}{% set col_min_w = '115px' %}
|
{% elif col_xwidget == 'time-picker' %}{% set col_min_w = '115px' %}
|
||||||
{% elif col_xwidget == 'file-upload-single' %}{% set col_min_w = '200px' %}
|
{% elif col_xwidget == 'file-upload-single' %}{% set col_min_w = '200px' %}
|
||||||
@@ -525,7 +530,13 @@
|
|||||||
<tr class="array-table-row" data-index="{{ item_index }}">
|
<tr class="array-table-row" data-index="{{ item_index }}">
|
||||||
{% for col_name in display_columns %}
|
{% for col_name in display_columns %}
|
||||||
{% set col_def = item_properties.get(col_name, {}) %}
|
{% set col_def = item_properties.get(col_name, {}) %}
|
||||||
{% set col_type = col_def.get('type', 'string') %}
|
{# Normalize nullable types e.g. ["null","integer"] → "integer" #}
|
||||||
|
{% set _raw_type = col_def.get('type', 'string') %}
|
||||||
|
{% if _raw_type is iterable and _raw_type is not string %}
|
||||||
|
{% set col_type = (_raw_type | reject('equalto','null') | list | first) or 'string' %}
|
||||||
|
{% else %}
|
||||||
|
{% set col_type = _raw_type or 'string' %}
|
||||||
|
{% endif %}
|
||||||
{% set col_xwidget = col_def.get('x-widget') or col_def.get('x_widget', '') %}
|
{% set col_xwidget = col_def.get('x-widget') or col_def.get('x_widget', '') %}
|
||||||
{% set col_enum = col_def.get('enum', []) %}
|
{% set col_enum = col_def.get('enum', []) %}
|
||||||
{% set col_value = item.get(col_name, col_def.get('default', '')) %}
|
{% set col_value = item.get(col_name, col_def.get('default', '')) %}
|
||||||
|
|||||||
Reference in New Issue
Block a user