mirror of
https://github.com/Donkie/Spoolman.git
synced 2026-10-21 17:42:16 -05:00
Donkie
7d683eb246
Two lower-severity findings from the audit (todo.md tasks 6 and 7). A spreadsheet executes a cell that begins with =, +, -, @, tab or CR, so a vendor named `=cmd|' /C calc'!A0` runs when someone opens the CSV export (CWE-1236). Such cells are now prefixed with a single quote, which spreadsheets read as "this is literal text" and do not display. Only strings are escaped -- numbers reach the writer as numeric types, so a negative number cannot be mistaken for a formula and must not grow a stray quote. Header names need no escaping: they are fixed attribute names or extra-field keys, which are already constrained to ^[a-z0-9_]+$. Exports also gained Content-Disposition, so they download under a sensible name rather than rendering in the browser. Extra-field values had no size limit and the columns are Text(), so a single request could store megabytes: unbounded database growth, and the resulting websocket event overflowed the 1 MiB default frame limit, which killed live updates for every connected client. Values are now capped at the same 65535 characters as settings. The number of extra fields per entity is capped too -- the per-entity key count was already bounded, since unknown keys are rejected, but the registry itself could be grown without limit, and it is cached in memory and embedded in every entity response. Finally, a setting larger than the column limit returned 500; the ValueError is now caught and returned as a 400. Verified end-to-end: a 2 MB extra value is refused on both create and update, an oversized setting returns 400 and leaves the setting unset, normal values still work, and a vendor named `=cmd|' /C calc'!A0` exports as `'=cmd|' /C calc'!A0` while the JSON export keeps the raw name.
48 lines
1.2 KiB
Python
48 lines
1.2 KiB
Python
"""Tests for CSV formula injection in exports.
|
|
|
|
A spreadsheet executes a cell that starts with `=`, so a vendor named `=cmd|' /C calc'!A0` runs
|
|
when someone opens the exported file (CWE-1236). The attacker only needs to get the string into
|
|
the database; the victim is whoever opens the export.
|
|
"""
|
|
|
|
import pytest
|
|
|
|
from spoolman.export import escape_csv_value
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"value",
|
|
[
|
|
"=cmd|' /C calc'!A0",
|
|
"=1+1",
|
|
"+1",
|
|
"-1+1",
|
|
"@SUM(A1)",
|
|
"\tleading tab",
|
|
"\rleading carriage return",
|
|
],
|
|
)
|
|
def test_formula_like_values_are_escaped(value: str):
|
|
assert escape_csv_value(value) == "'" + value
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"value",
|
|
[
|
|
"Prusament",
|
|
"eSUN PLA+",
|
|
"",
|
|
"a = b",
|
|
"1+1",
|
|
"spool-2",
|
|
],
|
|
)
|
|
def test_ordinary_values_are_untouched(value: str):
|
|
assert escape_csv_value(value) == value
|
|
|
|
|
|
@pytest.mark.parametrize("value", [-1, -1.5, 0, 42, None, True])
|
|
def test_non_strings_are_untouched(value: object):
|
|
"""Numbers reach the writer as numeric types, so a negative number must not grow a quote."""
|
|
assert escape_csv_value(value) is value
|