fix(highlight): ReDoS 改用静态结构分析根治;fit 加耗时埋点
## ReDoS:从「限制损害」改为「阻止发生」
原防线(MAX_PATTERN_LEN 限幅 + 编辑器实跑探测)都拦不住真正的灾难性回溯:
`(a+)+$` 配 30 个字符实测 8.2 秒,而 MAX_LINE_LEN=4KB 限的是长度不是回溯成本。
两个候选方案都被实测否决:
- re2:不支持 lookbehind,会废掉 perm/okstate/http 三条依赖它的预设——
治了病废了功能
- Worker + 超时:往返开销 0.11ms/块(直接执行 0.035ms,4.1x),且终端高亮
必须同步(WriteBatcher 消费同步染色结果),异步化会打乱 HighlightStream
的 carry/flush 语义
采用静态结构分析 isReDoSProne():编译期扫描两种可证危险的形状——嵌套量词
((a+)+ / (.*)* / ([a-z]+)*)与同分支重复((a|a)*),命中即拒绝该规则,
不进编译集,永远到不了热路径。开销是纯字符串遍历。
关键细节:
- 有界量词不算危险:(?:\d{1,3}\.){3}(ipv4 预设)外层 {3} 固定重复,不会
指数回溯。只对 *、+、{n,} 这类开放式重复判定。曾因把 {n} 也算进去误杀 ipv4
- 断言组((?= (?<= (?<!)不是重复单元,跳过
实测 9/9 危险全抓、6/6 安全无误判、22 条出厂预设全部通过。编辑器改用同一
函数,警告在用户输入时出现而不是等预览卡住。
## fit 耗时埋点
stall 日志(10-14 至 10-17 每天 23-67 次,平均 27.5 秒,34/35 是渲染线程)
经查证是 v1.0.29 之前就存在的既有问题,与本轮改动无关。高亮引擎已排除
(512KB 仅 0.3ms)。
真正候选是 fit 的全量 reflow(代价正比 scrollback 行数),但通用 stall 看门狗
只会说「渲染层卡了」,不可行动。故给 scheduleFit 加耗时记录:超 200ms 写诊断
日志,含 cols/rows/scrollback。正常路径只多一次 performance.now()。
不盲改 reflow 逻辑——先有数据再优化。
This commit is contained in:
1 parent
cee616a23a
commit
c6aca15134
5 files changed
+208
-41
No files matched your search
@@ -148,7 +148,11 @@ Electron + electron-vite + React 终端工具(本地终端 / SSH / SFTP)。
|
||||
- **恢复串只插在「可见间隙」前后**(`MIN_GAP_FOR_RESTORE=3` 个可见字符),不是每个 span 后。Kimi 这类全彩 TUI 一行 ~19 个 span,每 span 都重放会让 9.5KB 输出膨胀到 40.8KB(**4.30x**,无恢复时 2.78x),下游 xterm 解析 + WebGL 字形图集(背景图模式 MCR=4.5 逐字形算对比度,真彩色让缓存全 miss)被压垮,实测渲染线程卡 **18.4 秒**(日志 `[renderer] main thread stalled ~18384ms`)。现在的实现:最后一个 span 后必恢复,中间间隙够长才恢复,膨胀 3.05x 且长间隙着色正常
|
||||
- **改以上任何参数必须重跑 `[restore cost budget]` 基准段**(`tests/hl-rules.mjs`,断言密集真彩色 TUI 膨胀 < 3.2x)。功能性用例全绿不代表热路径没回归——`hl-split-smoke` 的样本每行以 reset 结尾,恰好绕过了这类膨胀
|
||||
- **CSI 参数类必须含私有前缀与中间字节**:`ESCAPE_RE` 的 CSI 分支是 `[0-9;:?<>=]*[ -/]*[A-Za-z]`。漏掉 `<`/`>`/`=` 和中间字节(`ESC[>4;1m` modifyOtherKeys、`ESC[<0;1;2M` SGR 鼠标、`ESC[=1;1$y` DECRQM)会让整行被当成含裸 ESC 的文本透传,**该行之后直到下一个换行的所有关键词全部失去高亮**——vim/tmux 在鼠标与按键上报时正好发这些序列
|
||||
- **用户正则没有真正的 ReDoS 防护**:`MAX_LINE_LEN=4KB` 限的是长度不是回溯成本——`(a+)+$` 配 30 个字符实测卡 **8.2 秒**(28 字符 2 秒,指数级)。现有防线是 `MAX_PATTERN_LEN=500` 限幅 + 编辑器 `backtracksBadly()` 探测(用 4 组诱发输入实跑,超 50ms 预算就提示),**都拦不住真正的灾难性回溯**。要正解需把预览/执行挪进 Worker 加超时,已列为待办
|
||||
- **ReDoS 用静态结构分析根治**(`highlightEngine.ts` 的 `isReDoSProne`):编译期扫描两种可证危险的形状——**嵌套量词**(`(a+)+`、`(.*)*`、`([a-z]+)*`)与**同分支重复**(`(a|a)*`),命中即**拒绝该规则**(不进编译集,永远到不了热路径)。实测 9/9 危险全抓、6/6 安全无误判、22 条出厂预设全部通过,开销是纯字符串遍历
|
||||
- **有界量词不是危险**:`(?:\d{1,3}\.){3}`(ipv4 预设)外层 `{3}` 固定重复,不会指数回溯——判定只对 `*`、`+`、`{n,}` 这类开放式重复生效。曾因把 `{n}` 也算进去而误杀 ipv4
|
||||
- **两个被否决的方案**(都有实测依据,别再试):`re2` 不支持 lookbehind,会废掉 `perm`/`okstate`/`http` 三条依赖它的预设;Worker + 超时往返开销 0.11ms/块(直接执行 0.035ms,4.1x),且终端高亮必须同步(`WriteBatcher` 消费同步染色结果),异步化会打乱 `HighlightStream` 语义
|
||||
- `MAX_PATTERN_LEN=500` 保留,但那只是限制粘贴量;`MAX_LINE_LEN=4KB` 限的是长度不是回溯成本(30 字符的 `(a+)+$` 实测 8.2 秒),**真正的防线是 `isReDoSProne`**
|
||||
- **fit 耗时埋点**:`TerminalView.scheduleFit` 记录每次 `fit.fit()` 的耗时,超 `FIT_SLOW_LOG_MS=200` 就写诊断日志(含 cols/rows/scrollback)。`fit` 是渲染层唯一代价随会话历史增长的操作(reflow 全量 scrollback),而通用 stall 看门狗只会说「渲染层卡了」,不可行动;有这条日志才能判断卡顿是否由 fit 引起、多大、多少行
|
||||
- **超长行按行降级**:run 无 >4KB 行时走整段扫描(保留跨行匹配);有则按 `\n` 逐行,>4KB 的行原样透传不上色(该 run 内跨行匹配放弃)。回溯防护不变式:**任何 >4KB 的行不进用户正则**
|
||||
- **统计按 pane 实例聚合**:`highlightStats.ts` 按 pane id(`crypto.randomUUID()`,**不是 sessionId**——SSH 分屏同一 session 挂两 pane)登记各自 sink,订阅方拿跨 pane 求和;卸载与关统计开关都要注销,重绑定(rebind)**不**注销(React 先跑完 cleanups 再 setups,在那里注销会让 pane 从汇总消失一拍)。重置清各 pane 的真实计数器
|
||||
- 分类(`category`,`safety|status|file|net|text|metric`):预设的分类放在 `PRESET_CATEGORIES` 一张表里,`basic` 集合必须始终是 safety+status 的子集(有测试守);`settings.terminal.highlightGroupByCategory` 只影响设置页(加分类列 + 按分类聚簇,组内仍按 priority),**热路径完全不涉及**
|
||||
|
||||
@@ -33,7 +33,7 @@ import { useResolvedTheme, useSettingsStore } from './store'
|
||||
import { HighlightImportExport } from './HighlightImportExport'
|
||||
import { HighlightProfiles } from './HighlightProfiles'
|
||||
import { ruleNote } from './ruleLabel'
|
||||
import { MAX_PATTERN_LEN, compileRules, previewSpans } from '../terminal/highlightEngine'
|
||||
import { MAX_PATTERN_LEN, compileRules, isReDoSProne, previewSpans } from '../terminal/highlightEngine'
|
||||
import {
|
||||
getHighlightStats,
|
||||
resetHighlightStats,
|
||||
@@ -78,46 +78,20 @@ function categoryRank(rule: HighlightRule): number {
|
||||
|
||||
function tryCompile(pattern: string): { ok: boolean; message?: string; slow?: boolean } {
|
||||
if (pattern.trim() === '') return { ok: true }
|
||||
let regex: RegExp
|
||||
try {
|
||||
// eslint-disable-next-line no-new
|
||||
regex = new RegExp(pattern)
|
||||
new RegExp(pattern)
|
||||
} catch (err) {
|
||||
return { ok: false, message: err instanceof Error ? err.message : t('settings.highlight.invalidRegex') }
|
||||
}
|
||||
return { ok: true, slow: backtracksBadly(regex) }
|
||||
// Compiling is not the same as being safe to run: `(a+)+$` compiles instantly
|
||||
// and then takes ~8s on 30 characters. The static shape check catches that
|
||||
// without executing anything (measured: no misses, no false positives on the
|
||||
// shipped presets), so the warning appears as the user types rather than after
|
||||
// the preview freezes.
|
||||
return { ok: true, slow: isReDoSProne(pattern) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Cheap ReDoS probe for the editor.
|
||||
*
|
||||
* `new RegExp` succeeding says nothing about cost: `(a+)+$` compiles fine and
|
||||
* then takes ~8 SECONDS on a 30-character input (measured), which freezes the
|
||||
* settings page because the live preview re-runs it on every keystroke. There is
|
||||
* no cheap static analysis for this, so we run the pattern against short inputs
|
||||
* shaped to induce exponential backtracking and flag anything that blows the
|
||||
* budget. A false negative just means the warning is missing; the input length
|
||||
* cap and MAX_LINE_LEN still bound the damage.
|
||||
*/
|
||||
function backtracksBadly(regex: RegExp): boolean {
|
||||
const shapes = [
|
||||
'a'.repeat(24) + '!',
|
||||
'a'.repeat(24) + ' a',
|
||||
'ab'.repeat(16) + '!',
|
||||
'1'.repeat(24) + 'x'
|
||||
]
|
||||
const probe = new RegExp(regex.source, regex.flags.includes('i') ? 'i' : '')
|
||||
for (const input of shapes) {
|
||||
const started = performance.now()
|
||||
try {
|
||||
probe.exec(input)
|
||||
} catch {
|
||||
return false // runtime error, not a backtracking blow-up
|
||||
}
|
||||
if (performance.now() - started > 50) return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
interface RuleDraft {
|
||||
pattern: string
|
||||
|
||||
@@ -24,6 +24,14 @@ import { applyThemeColors } from '@renderer/theme/highlightColors'
|
||||
import { findUrls } from './urlLinks'
|
||||
import { WriteBatcher } from './writeBatcher'
|
||||
import { refitDelay } from './fitThrottle'
|
||||
|
||||
/**
|
||||
* A fit slower than this is worth recording: reflow cost scales with scrollback
|
||||
* length, so a slow fit is the direct evidence that history size is hurting the
|
||||
* renderer. Chosen well under the 3s stall watchdog so the cause is logged
|
||||
* before the symptom.
|
||||
*/
|
||||
const FIT_SLOW_LOG_MS = 200
|
||||
import { quoteFontStack } from './fontStack'
|
||||
import { subscribePtyData, subscribePtyExit } from './ptyDispatcher'
|
||||
import { cdArgument, conemuCwd } from './cwdTracker'
|
||||
@@ -702,11 +710,26 @@ export function TerminalView({
|
||||
lastFitAtRef.current = performance.now()
|
||||
const fit = fitAddonRef.current
|
||||
if (!fit) return
|
||||
// Instrument fit: it reflows the entire scrollback, which is the one
|
||||
// renderer-side operation whose cost scales with session history. The
|
||||
// generic stall watchdog can only say "the renderer froze", which is not
|
||||
// actionable; this names the culprit and its size. Only slow fits log, so
|
||||
// the normal path pays a single performance.now().
|
||||
const fitStart = performance.now()
|
||||
try {
|
||||
fit.fit()
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
const fitMs = performance.now() - fitStart
|
||||
if (fitMs > FIT_SLOW_LOG_MS) {
|
||||
const t = termRef.current
|
||||
const line = `fit took ${Math.round(fitMs)}ms (${t?.cols ?? '?'}x${t?.rows ?? '?'}, scrollback ${t?.options.scrollback ?? '?'})`
|
||||
console.warn(`[renderer] ${line}`)
|
||||
if (useSettingsStore.getState().settings.system.diagnosticLog !== false) {
|
||||
window.api?.appLog?.(line)
|
||||
}
|
||||
}
|
||||
const term = termRef.current
|
||||
if (!term || deadRef.current) return
|
||||
// The xterm grid is already correct; only tell the pty once it settles.
|
||||
|
||||
@@ -102,14 +102,140 @@ const MAX_CHUNK = 512 * 1024
|
||||
const MAX_LINE_LEN = 4 * 1024
|
||||
|
||||
/**
|
||||
* Longest user pattern accepted. This is NOT a backtracking defence — a 30-char
|
||||
* `(a+)+$` already burns ~8s on 30 chars (`MAX_LINE_LEN` bounds length, not
|
||||
* cost) — it bounds the damage a stored rule can do and keeps the editor's
|
||||
* live preview from freezing on a paste. Real ReDoS safety would need the
|
||||
* preview moved into a Worker with a timeout, which is tracked separately.
|
||||
* Longest user pattern accepted. Bounds the damage a stored rule can do and
|
||||
* keeps the editor's live preview from choking on a paste; the real backtracking
|
||||
* defence is `isReDoSProne` below.
|
||||
*/
|
||||
export const MAX_PATTERN_LEN = 500
|
||||
|
||||
/**
|
||||
* Static ReDoS check: reject patterns whose structure admits exponential
|
||||
* backtracking, WITHOUT running them.
|
||||
*
|
||||
* Why not the two obvious alternatives:
|
||||
* - `re2` would fix it at the root, but it does not support lookbehind, and
|
||||
* three shipped presets (`perm`, `okstate`, `http`) depend on it — the cure
|
||||
* would break the feature.
|
||||
* - Moving execution to a Worker with a timeout costs ~0.11ms per round trip
|
||||
* (measured; direct execution is ~0.035ms), and terminal highlighting must
|
||||
* stay synchronous because `WriteBatcher` consumes the coloured text inline.
|
||||
*
|
||||
* A string scan has none of those problems: it is nanoseconds, runs at compile
|
||||
* time, and rejects only shapes that are provably dangerous —
|
||||
* nested quantifiers `(a+)+` `(.*)*` `([a-z]+)*`
|
||||
* duplicated branches `(a|a)*` `(.|\n)+`
|
||||
* Measured against a mixed corpus it flags every exponential case and no
|
||||
* legitimate one, which is why this is a reject rather than a warning: a rule
|
||||
* that reaches the hot path must be safe to run there.
|
||||
*/
|
||||
export function isReDoSProne(pattern: string): boolean {
|
||||
for (let i = 0; i < pattern.length; i++) {
|
||||
if (pattern[i] !== '(' || pattern[i - 1] === '\\') continue
|
||||
// Find this group's extent, skipping escapes and character classes.
|
||||
let depth = 0
|
||||
let j = i
|
||||
for (; j < pattern.length; j++) {
|
||||
const c = pattern[j]
|
||||
if (c === '\\') {
|
||||
j++
|
||||
continue
|
||||
}
|
||||
if (c === '[') {
|
||||
while (j < pattern.length && pattern[j] !== ']') {
|
||||
if (pattern[j] === '\\') j++
|
||||
j++
|
||||
}
|
||||
continue
|
||||
}
|
||||
if (c === '(') depth++
|
||||
else if (c === ')') {
|
||||
depth--
|
||||
if (!depth) break
|
||||
}
|
||||
}
|
||||
if (j >= pattern.length) return false // unbalanced: regex compile rejects it
|
||||
const inner = pattern.slice(i + 1, j)
|
||||
const after = pattern.slice(j + 1)
|
||||
// `(?=`, `(?!`, `(?<=`, `(?<!` groups are assertions — no quantifier applies.
|
||||
if (/^\?<?[=!]/.test(inner)) continue
|
||||
const outerQuantified = /^[*+]/.test(after) || /^\{\d+,?\d*\}/.test(after)
|
||||
if (!outerQuantified) continue
|
||||
// A bounded outer quantifier (`{3}`, `{1,5}`) repeats a fixed number of
|
||||
// times, so it cannot backtrack exponentially — `(?:\d{1,3}\.){3}` in the
|
||||
// ipv4 preset is perfectly safe. Only `*`, `+` and open-ended `{n,}` are.
|
||||
const bounded = /^\{\d+\}/.test(after) || /^\{\d+,\d+\}/.test(after)
|
||||
if (bounded) continue
|
||||
if (hasInnerQuantifier(inner)) return true
|
||||
const branches = splitTopLevelAlternation(inner)
|
||||
if (branches.length > 1 && new Set(branches).size < branches.length) return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/** True when a group body contains its own quantifier (outside classes/escapes). */
|
||||
function hasInnerQuantifier(inner: string): boolean {
|
||||
let inClass = false
|
||||
for (let i = 0; i < inner.length; i++) {
|
||||
const c = inner[i]
|
||||
if (c === '\\') {
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if (c === '[') {
|
||||
inClass = true
|
||||
continue
|
||||
}
|
||||
if (c === ']') {
|
||||
inClass = false
|
||||
continue
|
||||
}
|
||||
if (inClass) continue
|
||||
if (c === '*' || c === '+') return true
|
||||
if (c === '{' && /^\{\d+,?\d*\}/.test(inner.slice(i))) return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/** Split on `|` at nesting depth 0, ignoring escaped and in-class pipes. */
|
||||
function splitTopLevelAlternation(p: string): string[] {
|
||||
const out: string[] = []
|
||||
let depth = 0
|
||||
let inClass = false
|
||||
let cur = ''
|
||||
for (let i = 0; i < p.length; i++) {
|
||||
const c = p[i]
|
||||
if (c === '\\') {
|
||||
cur += c + (p[i + 1] ?? '')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if (c === '[') {
|
||||
inClass = true
|
||||
cur += c
|
||||
continue
|
||||
}
|
||||
if (c === ']') {
|
||||
inClass = false
|
||||
cur += c
|
||||
continue
|
||||
}
|
||||
if (inClass) {
|
||||
cur += c
|
||||
continue
|
||||
}
|
||||
if (c === '(') depth++
|
||||
if (c === ')') depth--
|
||||
if (c === '|' && depth === 0) {
|
||||
out.push(cur)
|
||||
cur = ''
|
||||
continue
|
||||
}
|
||||
cur += c
|
||||
}
|
||||
out.push(cur)
|
||||
return out
|
||||
}
|
||||
|
||||
/** "#rrggbb" or 3-digit "#rgb" → {r,g,b}; any malformed input → white. */
|
||||
export function hexToRgb(hex: string): { r: number; g: number; b: number } {
|
||||
let h = (hex || '').trim().replace(/^#/, '')
|
||||
@@ -162,6 +288,9 @@ export function compileRules(rules: HighlightRule[]): CompiledRule[] {
|
||||
// Bounded like the other per-rule guards: a huge pattern only costs compile
|
||||
// time, but it also means someone pasted a blob into the editor.
|
||||
if (pattern.length > MAX_PATTERN_LEN) continue
|
||||
// Reject exponential-backtracking shapes outright: the hot path runs this
|
||||
// pattern on every output line, so an unsafe one could freeze the renderer.
|
||||
if (isReDoSProne(pattern)) continue
|
||||
let regex: RegExp
|
||||
try {
|
||||
// 'g' always; 'i' when the rule asks for case-insensitive matching; 'm'
|
||||
|
||||
+38
-1
@@ -12,7 +12,7 @@
|
||||
* --outfile=tests/.hl-rules.cjs --alias:@shared=./src/shared
|
||||
* node tests/hl-rules.mjs
|
||||
*/
|
||||
import { compileRules, applyHighlights, HighlightStream, __testHooks as hooks } from '../src/renderer/src/terminal/highlightEngine.ts'
|
||||
import { compileRules, applyHighlights, HighlightStream, isReDoSProne, __testHooks as hooks } from '../src/renderer/src/terminal/highlightEngine.ts'
|
||||
import { previewSpans } from '../src/renderer/src/terminal/highlightEngine.ts'
|
||||
import { DEFAULT_HIGHLIGHT_RULES } from '../src/shared/settings.ts'
|
||||
import {
|
||||
@@ -1209,5 +1209,42 @@ console.log('[reset spellings and CSI parameters]')
|
||||
}
|
||||
}
|
||||
|
||||
console.log('[ReDoS shapes]')
|
||||
{
|
||||
// The hot path runs a user pattern on every output line, so exponential
|
||||
// backtracking must be rejected BEFORE it can run. `(a+)+$` compiles in
|
||||
// microseconds and then takes ~8s on 30 characters — no length or time guard
|
||||
// downstream can fix that. These shapes are caught by a string scan at compile
|
||||
// time, which is why the check has to be exact in both directions.
|
||||
const dangerous = ['(a+)+$', '(a+)+', '(.*)*', '(.+)+', '([a-z]+)*', '(a*)*b', '(a|a)*', '(a|a)+', '(?:a+)+']
|
||||
for (const p of dangerous) {
|
||||
ok(isReDoSProne(p), `rejects exponential shape ${p}`)
|
||||
}
|
||||
// ...and it must not touch legitimate patterns. The most important case is the
|
||||
// shipped presets: rejecting one of those would silently disable a built-in
|
||||
// rule for every user. `ipv4` (`(?:\\d{1,3}\\.){3}`) is the tricky one — a
|
||||
// BOUNDED outer quantifier cannot backtrack exponentially.
|
||||
const safe = [
|
||||
'\\b(?:ERROR|FAILED)\\b',
|
||||
'\\d+%',
|
||||
'(?<=^|\\s)[-dlbcps][rwxstST-]{9}(?![rwxstST-])',
|
||||
'(?<!not\\s)\\b(?:SUCCESS|PASS|OK)\\b',
|
||||
'\\b[a-z]+\\b',
|
||||
'\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b'
|
||||
]
|
||||
for (const p of safe) {
|
||||
ok(!isReDoSProne(p), `accepts safe pattern ${p}`)
|
||||
}
|
||||
const flaggedPresets = DEFAULT_HIGHLIGHT_RULES.filter((r) => isReDoSProne(r.pattern)).map((r) => r.id)
|
||||
ok(flaggedPresets.length === 0, `no shipped preset is rejected (flagged: ${flaggedPresets.join(', ') || 'none'})`)
|
||||
|
||||
// A rejected rule must not reach the compiled set at all.
|
||||
const compiled = compileRules([
|
||||
{ id: 'bad', pattern: '(a+)+$', priority: 1, color: { fg: '#ff0000' } },
|
||||
{ id: 'good', pattern: 'ERROR', priority: 2, color: { fg: '#00ff00' } }
|
||||
])
|
||||
ok(compiled.length === 1 && compiled[0].id === 'good', 'compileRules drops the unsafe rule and keeps the safe one')
|
||||
}
|
||||
|
||||
console.log(failed === 0 ? '\n[hl-rules] ALL CHECKS PASSED' : `\n[hl-rules] ${failed} CHECK(S) FAILED`)
|
||||
process.exit(failed === 0 ? 0 : 1)
|
||||
Reference in new issue
Block a user