Fix an inefficient regex in autoInject (#1767)

* Fix an inefficient regex in autoInject * 'properly strip comments in argument definitions' test failure * Update test/autoInject.js Co-authored-by: Rich Trott <[email protected]> * Update on url-comments lib/autoInject.js Co-authored-by: Rich Trott <[email protected]> * move new tests test/autoInject.js * indentation fix test/autoInject.js Co-authored-by: Rich Trott <[email protected]>
caolan · Dec 2, 2021 · cdfb491 · cdfb491
1 parent bb41f2a
commit cdfb491
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 1 deletion.
diff --git a/lib/autoInject.js b/lib/autoInject.js
@@ -6,7 +6,7 @@ var FN_ARGS = /^(?:async\s+)?(?:function)?\s*\w*\s*\(\s*([^)]+)\s*\)(?:\s*{)/;
 var ARROW_FN_ARGS = /^(?:async\s+)?\(?\s*([^)=]+)\s*\)?(?:\s*=>)/;
 var FN_ARG_SPLIT = /,/;
 var FN_ARG = /(=.+)?(\s*)$/;
-var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
+var STRIP_COMMENTS = /(\/\*(?:[^/]|\/(?!\*))*\*\/)|\/\/.*$/mg;
 
 function parseParams(func) {
     const src = func.toString().replace(STRIP_COMMENTS, '');

diff --git a/test/autoInject.js b/test/autoInject.js
@@ -224,4 +224,33 @@ describe('autoInject', () => {
             done()
         })
     })
+
+    it('should not be subject to ReDoS', () => {
+        // This test will timeout if the bug is present.
+        var someComments = 'text/*'.repeat(1000000)
+        expect(() => async.autoInject({
+            someComments,
+            a () {}
+        })).to.throw()
+    });
+
+    it('should properly strip comments in argument definitions', (done) => {
+        async.autoInject({
+            task1: function(task2, /* ) */ callback) {
+                callback(null, true);
+            },
+            task2: function task2(task3 // )
+                ,callback) {
+                callback(null, true);
+            },
+            task3: function task3(callback) {
+                callback(null, true);
+            }
+        },
+        (err, result) => {
+            expect(err).to.eql(null);
+            expect(result).to.deep.eql({task1: true, task2: true, task3: true});
+            done();
+        });
+    });
 });