Skip to content

Commit

Permalink
Fix regexp in utils.stackTraceFilter to prevent ReDoS #3416 (#3686)
Browse files Browse the repository at this point in the history
When the stacktrace begins with a large `Error.message` (single or multi line), Mocha's stack prettifier method (enabled by default) takes a long time to finish due to its regular expression. Certain assertions (e.g., `contains`, `deeplyEqual`) against large objects can trigger this problem.
This PR alters the RegExp to skip over the error message portion of the stacktrace.
  • Loading branch information
cyjake authored and plroebuck committed Jan 30, 2019
1 parent 7fee3a3 commit 1a43d8b
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 3 deletions.
4 changes: 1 addition & 3 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,6 @@ exports.stackTraceFilter = function() {
function isMochaInternal(line) {
return (
~line.indexOf('node_modules' + slash + 'mocha' + slash) ||
~line.indexOf('node_modules' + slash + 'mocha.js') ||
~line.indexOf('bower_components' + slash + 'mocha.js') ||
~line.indexOf(slash + 'mocha.js')
);
}
Expand Down Expand Up @@ -701,7 +699,7 @@ exports.stackTraceFilter = function() {
}

// Clean up cwd(absolute)
if (/\(?.+:\d+:\d+\)?$/.test(line)) {
if (/:\d+:\d+\)?$/.test(line)) {
line = line.replace('(' + cwd, '(');
}

Expand Down
57 changes: 57 additions & 0 deletions test/unit/runner.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var Runner = mocha.Runner;
var Test = mocha.Test;
var Hook = mocha.Hook;
var path = require('path');
var fs = require('fs');
var noop = mocha.utils.noop;

describe('Runner', function() {
Expand Down Expand Up @@ -496,6 +497,62 @@ describe('Runner', function() {
runner.failHook(hook, err);
});
});

describe('hugeStackTrace', function() {
beforeEach(function() {
if (path.sep !== '/') {
this.skip();
}
});

it('should not hang if the error message is ridiculously long single line', function(done) {
var hook = new Hook();
hook.parent = suite;
var data = [];
// mock a long message
for (var i = 0; i < 10000; i++) data[i] = {a: 1};
var message = JSON.stringify(data);
var err = new Error();
// Fake stack-trace
err.stack = [message].concat(stack).join('\n');

runner.on('fail', function(hook, err) {
expect(
err.stack
.split('\n')
.slice(1)
.join('\n'),
'to be',
stack.slice(0, 3).join('\n')
);
done();
});
runner.failHook(hook, err);
});

it('should not hang if error message is ridiculously long multiple lines either', function(done) {
var hook = new Hook();
hook.parent = suite;
var fpath = path.join(__dirname, '../../mocha.js');
var message = fs.readFileSync(fpath, 'utf8');
var err = new Error();
// Fake stack-trace
err.stack = [message].concat(stack).join('\n');

runner.on('fail', function(hook, err) {
expect(
err.stack
.split('\n')
.slice(-3)
.join('\n'),
'to be',
stack.slice(0, 3).join('\n')
);
done();
});
runner.failHook(hook, err);
});
});
});

describe('abort', function() {
Expand Down

0 comments on commit 1a43d8b

Please sign in to comment.