hooks: sed labels end at semicolon — commands after b/t/T/: are still scanned

CodeRabbit security finding on the merged sed work (CWE-693, verified live
on main via #3495): GNU sed labels for b, t, T and : end at ';' or newline —
verified against GNU sed 4.9 ('bar;e cmd' parses label 'a' and still runs
cmd; '}' is not a label terminator). Reading those operands to end of line
let 'sed :a;e git commit --no-verify' pass the hook while sed executed the
e command.

Labels are now read up to ';' or newline and command parsing resumes after
them. Text, filename and quit operands (a, c, i, r, w, q, Q) keep the
end-of-line reading: nothing after them on the line executes.

5 regression tests pin the three bypass paths and the benign boundary
(a bare label and a bare branch stay data). Suite: 817/817.
This commit is contained in:
chenhz01
2026-10-11 18:52:44 +08:00
parent 3af000e143
commit cffcf05e7a
2 changed files with 23 additions and 2 deletions
+13 -1
View File
@@ -586,6 +586,17 @@ function readSedLineOperand(script, i, budget) {
return { end, parts: [], stdinExec: false };
}
// Branch and label commands (b, t, T, :) take a label that ends at `;` or
// newline — verified against GNU sed 4.9: ':a;e cmd' and 'ba;e cmd' both
// parse the label up to `;` and still run the following `e`. Commands after
// the label therefore continue to be scanned, so return the label end and
// let the caller resume command parsing there.
function readSedLabel(script, i, budget) {
let end = i + 1;
while (end < script.length && script[end] !== ';' && script[end] !== '\n') { budget.spend(); end++; }
return { end, parts: [], stdinExec: false };
}
function readSedCommand(script, i, budget) {
const letter = script[i];
if (letter === 's') {
@@ -602,7 +613,8 @@ function readSedCommand(script, i, budget) {
return sub ? { end: sub.end, parts: [], stdinExec: false } : null;
}
if (letter === 'e') return readSedExecute(script, i, budget);
if ('acirwWb:tTqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
if ('btT:'.includes(letter)) return readSedLabel(script, i, budget);
if ('acirwWqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
if ('dDgGhHlnNpPxFzv='.includes(letter)) return { end: i + 1, parts: [], stdinExec: false };
return null; // unmodeled command letter: keep the opaque default
}
+10 -1
View File
@@ -2183,7 +2183,16 @@ const sedCases = [
['unknown escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\git commit --no-verify/e'", 2],
['escaped-newline continued e operand is conservatively blocked', "printf x | sed -e 'e git commit \\\n--no-verify'", 2],
['octal escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\147it commit --no-verify/e'", 2],
['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0]
['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0],
// Labels end at `;` and following commands still execute (GNU sed 4.9
// verified: ':a;e cmd' runs cmd). Reading label operands to end of line
// let an e command hide behind a label (CodeRabbit finding, landed via
// #3495) — the label reader must stop at `;` so the rest is scanned.
['e command after a label is scanned', "sed ':a;e git commit --no-verify'", 2],
['e command after a b branch is scanned', "printf x | sed 'ba;e git commit --no-verify'", 2],
['e command after a t branch is scanned', "printf x | sed 'tX;e git commit --no-verify'", 2],
['plain label without following commands stays data', "sed ':a' 'git commit --no-verify'", 0],
['b branch alone stays data', "sed 'bx' 'git commit --no-verify'", 0]
];
for (const [name, command, expected] of sedCases) {
if (test(`sed model ${name}`, () => {