diff --git a/test/test_composer_code_injection.py b/test/test_composer_code_injection.py index a9527098..2b945d2f 100644 --- a/test/test_composer_code_injection.py +++ b/test/test_composer_code_injection.py @@ -67,7 +67,7 @@ def _module_level_code(src): tree = ast.parse(src) out = [] for node in tree.body: - if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.ImportFrom)): + if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.ImportFrom, ast.Import)): continue if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant): continue # the docstring @@ -145,6 +145,28 @@ def test_colour_channels_are_clamped_to_a_byte(): assert "(255, 0, 128)" in src, "channels were not clamped to 0-255" +#: fillR/fillG/fillB (and outR/G/B, bgR/G/B, trackR/G/B) go through +#: _as_fill_filter -> _as_rgb_filter, a separate path from _rgb_expr above. +#: It used raw int() until it was found to raise ValueError on a non-numeric +#: channel instead of clamping like every other coerced value in this module. +@pytest.mark.parametrize("evil", EXPR_PAYLOADS) +@pytest.mark.parametrize("channel", ["fillR", "fillG", "fillB"]) +def test_a_non_numeric_fill_channel_cannot_reach_the_source(evil, channel): + el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8, + "fillR": 0, "fillG": 0, "fillB": 128} + el[channel] = evil + src = _generated(_payload(elements=[el])) + assert "__import__" not in src and "os.system" not in src + assert not _module_level_code(src) + + +def test_fill_channels_are_clamped_to_a_byte(): + el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8, + "fillR": 99999, "fillG": -5, "fillB": 128} + src = _generated(_payload(elements=[el])) + assert "(255, 0, 128)" in src, "fill channels were not clamped to 0-255" + + def test_the_generated_module_still_has_no_top_level_statements(): """The clean case: a normal payload produces only imports and a class.""" el = {"type": "text", "id": "t1", "x": 4, "y": 4, "text": "hi", diff --git a/web_interface/blueprints/composer.py b/web_interface/blueprints/composer.py index 6f9d68ee..6521b9b1 100644 --- a/web_interface/blueprints/composer.py +++ b/web_interface/blueprints/composer.py @@ -95,8 +95,10 @@ def _as_rgb_filter(val) -> str: if val is None: return 'None' # nosemgrep: not a Flask route -- a Jinja filter emitting a Python - # tuple literal, with every channel coerced by int(). - return f'({int(val[0])}, {int(val[1])}, {int(val[2])})' # nosemgrep + # tuple literal, with every channel coerced and clamped by _safe_int(). + return (f'({_safe_int(val[0], 0, 0, 255)}, ' # nosemgrep + f'{_safe_int(val[1], 0, 0, 255)}, ' + f'{_safe_int(val[2], 0, 0, 255)})') def _as_fill_filter(val) -> str: @@ -278,7 +280,7 @@ def _preprocess_elements(elements: list) -> list: x_anchor = el.get('xAnchor') or None y_anchor = el.get('yAnchor') or None - p['min_width'] = int(el.get('minWidth', 0) or 0) + p['min_width'] = _safe_int(el.get('minWidth', 0) or 0, 0, 0, 4096) if t in ('text', 'clock'): font_key = el.get('font', 'press_start') @@ -290,7 +292,7 @@ def _preprocess_elements(elements: list) -> list: p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height') font_size = _FONT_SIZE_MAP.get(font_key, 8) char_w = _FONT_CHAR_W.get(font_key, 8) - line_spacing = int(el.get('lineSpacing', 2)) + line_spacing = _safe_int(el.get('lineSpacing', 2), 2, 0, 256) y_expr = p['y_expr'] p['y2_expr'] = f"({y_expr}) + {font_size + line_spacing}" if t == 'text': @@ -377,8 +379,8 @@ def _preprocess_elements(elements: list) -> list: elif t == 'progress_bar': p['x_expr'] = _compute_pos_expr(el.get('x', 0), x_anchor, 'width') p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height') - p['bar_width'] = int(el.get('barWidth', 40)) - p['bar_height'] = int(el.get('barHeight', 6)) + p['bar_width'] = _safe_int(el.get('barWidth', 40), 40, 0, 4096) + p['bar_height'] = _safe_int(el.get('barHeight', 6), 6, 0, 4096) binding = el.get('binding', {}) p['binding_key'] = binding.get('key', '') p['fill_tuple'] = _rgb_tuple(el, ('r', 'g', 'b'), (100, 200, 100)) @@ -403,8 +405,8 @@ def _preprocess_elements(elements: list) -> list: p['y_expr'] = y_expr p['x2_expr'] = f"({x_expr}) + {w}" p['y2_expr'] = f"({y_expr}) + {h}" - p['start_angle'] = int(el.get('startAngle', 0)) - p['end_angle'] = int(el.get('endAngle', 270)) + p['start_angle'] = _safe_int(el.get('startAngle', 0), 0, -3600, 3600) + p['end_angle'] = _safe_int(el.get('endAngle', 270), 270, -3600, 3600) p['line_width'] = _safe_int(el.get('lineWidth'), 2, 1, 64) p['rgb_tuple'] = _rgb_expr(el, 255, 200, 0) p['blink'] = bool(el.get('blink', False)) @@ -445,7 +447,7 @@ def _preprocess_elements(elements: list) -> list: p['y_expr'] = y_expr p['x2_expr'] = f"({x_expr}) + {w}" p['y2_expr'] = f"({y_expr}) + {h}" - p['border_radius'] = int(el.get('borderRadius', 3)) + p['border_radius'] = _safe_int(el.get('borderRadius', 3), 3, 0, 128) fill = ( [el.get('fillR', 0), el.get('fillG', 80), el.get('fillB', 180)] if el.get('hasFill', True) else None @@ -473,9 +475,9 @@ def _preprocess_elements(elements: list) -> list: elif t == 'pips': p['x_expr'] = _compute_pos_expr(el.get('x', 0), x_anchor, 'width') p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height') - p['pip_count'] = max(1, int(el.get('count', 5))) - p['pip_size'] = max(1, int(el.get('pipSize', 4))) - p['pip_spacing'] = max(0, int(el.get('pipSpacing', 2))) + p['pip_count'] = _safe_int(el.get('count', 5), 5, 1, 256) + p['pip_size'] = _safe_int(el.get('pipSize', 4), 4, 1, 256) + p['pip_spacing'] = _safe_int(el.get('pipSpacing', 2), 2, 0, 256) p['show_empty'] = bool(el.get('showEmpty', True)) binding = el.get('binding', {}) p['binding_key'] = binding.get('key', '') @@ -488,10 +490,10 @@ def _preprocess_elements(elements: list) -> list: y_expr = _compute_pos_expr(el.get('y', 0), y_anchor, 'height') p['x_expr'] = x_expr p['y_expr'] = y_expr - p['bar_width_px'] = int(el.get('width', 40)) - p['bar_height_px'] = int(el.get('height', 12)) - p['bar_count'] = max(1, int(el.get('barCount', 8))) - p['bar_spacing'] = max(0, int(el.get('barSpacing', 1))) + p['bar_width_px'] = _safe_int(el.get('width', 40), 40, 0, 4096) + p['bar_height_px'] = _safe_int(el.get('height', 12), 12, 0, 4096) + p['bar_count'] = _safe_int(el.get('barCount', 8), 8, 1, 256) + p['bar_spacing'] = _safe_int(el.get('barSpacing', 1), 1, 0, 256) binding = el.get('binding', {}) p['binding_key'] = binding.get('key', '') p['fill_tuple'] = _rgb_tuple(el, ('r', 'g', 'b'), (80, 200, 120)) @@ -508,8 +510,8 @@ def _preprocess_elements(elements: list) -> list: p['y_expr'] = y_expr p['x2_expr'] = f"({x_expr}) + {w}" p['y2_expr'] = f"({y_expr}) + {h}" - p['start_angle'] = int(el.get('startAngle', 135)) - p['end_angle'] = int(el.get('endAngle', 45)) + p['start_angle'] = _safe_int(el.get('startAngle', 135), 135, -3600, 3600) + p['end_angle'] = _safe_int(el.get('endAngle', 45), 45, -3600, 3600) p['line_width'] = _safe_int(el.get('lineWidth'), 3, 1, 64) p['rgb_tuple'] = _rgb_expr(el, 80, 220, 80) track = ( @@ -532,8 +534,8 @@ def _preprocess_elements(elements: list) -> list: p['y_expr'] = _compute_pos_expr(el.get('y', 0), y_anchor, 'height') p['text'] = el.get('text', 'Scrolling text') p['char_w'] = _FONT_CHAR_W.get(font_key, 8) - p['gap'] = int(el.get('gap', 16)) - p['scroll_speed'] = max(1, int(el.get('scrollSpeed', 1))) + p['gap'] = _safe_int(el.get('gap', 16), 16, 0, 4096) + p['scroll_speed'] = _safe_int(el.get('scrollSpeed', 1), 1, 1, 256) p['direction'] = el.get('direction', 'left') # Data key stored in self._data for stateful scrolling across # display() calls. It is spliced UNQUOTED into variable names diff --git a/web_interface/static/v3/js/composer/composer-app.js b/web_interface/static/v3/js/composer/composer-app.js index 170203a8..7dd2230a 100644 --- a/web_interface/static/v3/js/composer/composer-app.js +++ b/web_interface/static/v3/js/composer/composer-app.js @@ -572,7 +572,7 @@ function composerApp() { link.download = (this.metadata.id || 'composer-design') + '.composer.json'; link.href = url; link.click(); - URL.revokeObjectURL(url); + setTimeout(() => URL.revokeObjectURL(url), 0); }, importDesign() { @@ -583,6 +583,9 @@ function composerApp() { const file = e.target.files[0]; if (!file) return; const reader = new FileReader(); + reader.onerror = () => { + this._setStatus('Failed to read file', 'error'); + }; reader.onload = (ev) => { try { const data = JSON.parse(ev.target.result); @@ -1326,7 +1329,7 @@ function composerApp() { const url = URL.createObjectURL(blob); const a = document.createElement('a'); a.href = url; a.download = `${this.metadata.id}.zip`; a.click(); - URL.revokeObjectURL(url); + setTimeout(() => URL.revokeObjectURL(url), 0); this.generateStatus = 'done'; this._setStatus('Plugin ZIP downloaded', 'success'); setTimeout(() => { if (this.generateStatus === 'done') this.generateStatus = 'idle'; }, 4000);