Skip to content

Commit

Permalink
fix regex in utils.stackTraceFilter to prevent ReDoS #3416
Browse files Browse the repository at this point in the history
if the stack trace begins with a large error message (>= 20k charactors), and user leaves `--full-trace` disabled, `utils.stackTraceFilter()` takes ages to finish. Large error messages is quite possible when user makes containment assertions such as `expect(content).to.contain(word)`.
  • Loading branch information
cyjake committed Jan 30, 2019
1 parent 7fee3a3 commit 38ee9d7
Show file tree
Hide file tree
Showing 2 changed files with 57 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
56 changes: 56 additions & 0 deletions test/unit/runner.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,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 = require('path').join(__dirname, '../../mocha.js');
var message = require('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 38ee9d7

Please sign in to comment.