From a51df6d9e2d31b44be9adb6bc8732517db6bf96b Mon Sep 17 00:00:00 2001 From: Andi Albrecht Date: Sat, 25 Jul 2026 08:52:26 +0200 Subject: [PATCH] Measure reindent offsets backwards to avoid quadratic CPU use ReindentFilter._get_offset() reports the column a token starts on and is called once per group. It rebuilt the statement prefix from the start of the statement on every call, so reindenting a list of N parenthesized tuples cost O(N^2). A ~12 KB payload sized just below MAX_GROUPING_TOKENS occupied a worker for seconds (CWE-1333, GHSA-cfqr-cjx5-5jcm). Measure the current line by walking backwards from the token and counting characters instead, which stops at the preceding line break rather than touching the whole statement. Formatting output is unchanged, verified byte for byte over the test corpus and 147 option combinations. Both vectors reaching the offset calculation are covered by the new benchmark: IN (...) tuple lists via _process_parenthesis() and _process_identifierlist(), VALUES lists via _process_values(). Co-Authored-By: Claude Opus 5 --- CHANGELOG | 5 ++ benchmarks/bench_reindent_offset.py | 99 +++++++++++++++++++++++++++++ sqlparse/filters/reindent.py | 66 +++++++++++++++---- 3 files changed, 157 insertions(+), 13 deletions(-) create mode 100644 benchmarks/bench_reindent_offset.py Index: sqlparse-0.2.4/benchmarks/bench_reindent_offset.py =================================================================== --- /dev/null +++ sqlparse-0.2.4/benchmarks/bench_reindent_offset.py @@ -0,0 +1,99 @@ +"""Reindentation offset benchmarks (GHSA-cfqr-cjx5-5jcm, CWE-1333). + +Measures ``format(sql, reindent=True)`` for SQL that stresses +``ReindentFilter._get_offset()``, which reports the column a token starts on. + +It used to rebuild the statement prefix from the start of the statement on +every call, and it is called once per group -- so reindenting a list of N +parenthesized tuples cost O(N^2). An attacker who controls SQL sent to this +opt-in path can size a tuple list to stay just below the grouping-token cap +(``MAX_GROUPING_TOKENS = 10000``), so grouping succeeds and the expensive +reindentation path is entered. A payload of ~12 KB then pinned a CPU for +seconds. + +Both shapes below reach the same offset calculation but through different +filters -- ``IN (...)`` tuple lists via ``_process_parenthesis()`` and +``_process_identifierlist()``, ``VALUES`` lists via ``_process_values()``. + +Run with: python benchmarks/bench_reindent_offset.py +""" + +import math +import signal +import time + +import sqlparse + +TIMEOUT_SECONDS = 60 + + +def _alarm_handler(signum, frame): + raise TimeoutError() + + +signal.signal(signal.SIGALRM, _alarm_handler) + + +def in_tuple_list_sql(n_tuples): + """WHERE ... IN with n_tuples tuples -- the shape from the advisory.""" + tuples = ', '.join(f'({i}, {i * 2})' for i in range(n_tuples)) + return f'SELECT a FROM t WHERE (col1, col2) IN ({tuples})' + + +def values_list_sql(n_tuples): + """INSERT ... VALUES with n_tuples tuples -- reaches _process_values().""" + tuples = ', '.join(f'({i})' for i in range(n_tuples)) + return f'INSERT INTO t VALUES {tuples}' + + +def measure(label, sql): + signal.alarm(TIMEOUT_SECONDS) + t0 = time.perf_counter() + status = 'OK' + try: + # The vulnerable, opt-in path: reindentation. + sqlparse.format(sql, reindent=True) + except sqlparse.exceptions.SQLParseError: + status = 'CAP' # grouping token/depth cap fired before reindenting + except TimeoutError: + status = 'TIMEOUT' + finally: + signal.alarm(0) + dt = time.perf_counter() - t0 + print(f' {status:8} {dt:8.3f} s {label} ({len(sql)} B)') + return dt + + +# Absolute wall-clock is host dependent, so compare growth instead: the work +# scales linearly with the number of tuples, so a patched build should grow +# with an exponent near 1 and the O(N^2) bug with an exponent near 2. Sizes +# stay below the grouping-token cap; a larger input is rejected quickly by the +# cap instead of reaching the reindentation path. +VECTORS = ( + ('IN-tuple list ', in_tuple_list_sql, (150, 300, 600, 1200)), + ('VALUES list ', values_list_sql, (250, 500, 1000, 1950)), +) + +verdicts = [] +for name, build, sizes in VECTORS: + print(f'{name.strip()} (format reindent=True):') + times = [measure(f'{name} n={n:<5}', build(n)) for n in sizes] + + if times[0] > 0 and times[-1] > 0: + exponent = math.log(times[-1] / times[0]) / math.log(sizes[-1] + / sizes[0]) + else: + exponent = 0.0 + print(f' n grew {sizes[-1] / sizes[0]:.1f}x; ' + f'time grew {times[-1] / times[0]:.1f}x; ' + f'empirical scaling exponent ~= {exponent:.2f}\n') + verdicts.append((name.strip(), exponent)) + +vulnerable = [name for name, exponent in verdicts if exponent >= 1.5] +if vulnerable: + print('EVOHUNT_REINDENT_DOS_VERIFIED: super-linear (>= quadratic) CPU ' + f'growth via {", ".join(vulnerable)} -> ReindentFilter._get_offset ' + 'DoS is present (unpatched).') +else: + print('Growth is ~linear for every vector -> the ReindentFilter offset ' + 'fix appears to be present.') Index: sqlparse-0.2.4/sqlparse/filters/reindent.py =================================================================== --- sqlparse-0.2.4.orig/sqlparse/filters/reindent.py +++ sqlparse-0.2.4/sqlparse/filters/reindent.py @@ -23,25 +23,65 @@ class ReindentFilter(object): self._curr_stmt = None self._last_stmt = None - def _flatten_up_to_token(self, token): - """Yields all tokens up to token but excluding current.""" - if token.is_group: - token = next(token.flatten()) - - for t in self._curr_stmt.flatten(): - if t == token: - break - yield t - @property def leading_ws(self): return self.offset + self.indent * self.width + def _current_line_len(self, token): + """Returns the width of what's already emitted on *token*'s line. + + The tokens preceding *token* are visited last one first, so the walk + stops at the line break that starts the current line. Rebuilding the + statement prefix from its start instead made every caller + O(statement), and the callers running once per group (tuple lists, + identifier lists) quadratic in the number of groups -- a CPU + exhaustion vector (GHSA-cfqr-cjx5-5jcm). The walk is inlined and + counts characters rather than collecting them: both matter, since a + line without any break still has to be measured token by token. + """ + length = 0 + node = token + while node is not self._curr_stmt and node.parent is not None: + parent = node.parent + # ``Token`` doesn't implement ``__eq__``, so ``index()`` is an + # identity lookup and safe against tokens sharing a value. + stack = parent.tokens[:parent.tokens.index(node)] + while stack: + prev_ = stack.pop() + if prev_.is_group: + stack.extend(prev_.tokens) + continue + + value = prev_.value + size = len(value) + if not size: + continue + lines = value.splitlines() + if len(lines) == 1 and len(lines[0]) == size: + # No break in here. ``splitlines()`` hands back the value + # itself in that case, so this costs a scan but no copy -- + # which is what keeps a long break-free line affordable. + length += size + continue + + # ``value`` holds the break that starts the current line. The + # sentinel keeps a trailing break from collapsing, so ``lines`` + # always has one entry more than the number of breaks. + lines = (value + '.').splitlines() + tail = len(lines[-1]) - 1 + length + if tail: + return tail + # Nothing but a break to our right, and ``splitlines()`` drops + # that empty line -- so the line to measure is the one before. + if len(lines) > 2: + return len(lines[-2]) + length = len(lines[0]) + node = parent + return length + def _get_offset(self, token): - raw = u''.join(map(text_type, self._flatten_up_to_token(token))) - line = (raw or '\n').splitlines()[-1] # Now take current offset into account and return relative offset. - return len(line) - len(self.char * self.leading_ws) + return self._current_line_len(token) - len(self.char * self.leading_ws) def nl(self, offset=0): return sql.Token(