patch: fixes security issue with non-escaping inputs [GHSL-2020-373]

mikaelbr · Mar 11, 2021 · 2cdb290 · 2cdb290
1 parent 93fa026
commit 2cdb290
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 15 deletions.
diff --git a/lib/utils.js b/lib/utils.js
@@ -270,9 +270,9 @@ module.exports.constructArgumentList = function(options, extra) {
   var keepNewlines = !!extra.keepNewlines;
   var wrapper = extra.wrapper === void 0 ? '"' : extra.wrapper;
 
-  var escapeFn = function(arg) {
+  var escapeFn = function escapeFn(arg) {
     if (isArray(arg)) {
-      return removeNewLines(arg.join(','));
+      return removeNewLines(arg.map(escapeFn).join(','));
     }
 
     if (!noEscape) {
@@ -285,9 +285,7 @@ module.exports.constructArgumentList = function(options, extra) {
   };
 
   initial.forEach(function(val) {
-    if (typeof val === 'string') {
-      args.push(escapeFn(val));
-    }
+    args.push(escapeFn(val));
   });
   for (var key in options) {
     if (

diff --git a/test/notify-send.js b/test/notify-send.js
@@ -63,23 +63,32 @@ describe('notify-send', function() {
     notifier.notify({ message: 'some\n "me\'ss`age`"' });
   });
 
-  it('should send additional parameters as --"keyname"', function(done) {
-    var expected = ['"title"', '"body"', '--icon', '"icon-string"'];
+  it('should escape array items as normal items', function(done) {
+    var expected = [
+      '"Hacked"',
+      '"\\`touch HACKED\\`"',
+      '--category',
+      '"foo\\`touch exploit\\`"'
+    ];
 
     expectArgsListToBe(expected, done);
     var notifier = new Notify({ suppressOsdCheck: true });
-    notifier.notify({ title: 'title', message: 'body', icon: 'icon-string' });
+    var options = JSON.parse(
+      `{
+        "title": "Hacked",
+        "message":["\`touch HACKED\`"],
+        "category": ["foo\`touch exploit\`"]
+      }`
+    );
+    notifier.notify(options);
   });
 
-  it('should only include strings as arguments', function(done) {
-    var expected = ['"HACKED"'];
+  it('should send additional parameters as --"keyname"', function(done) {
+    var expected = ['"title"', '"body"', '--icon', '"icon-string"'];
 
     expectArgsListToBe(expected, done);
     var notifier = new Notify({ suppressOsdCheck: true });
-    var options = JSON.parse(
-      '{"title":"HACKED", "message":["`touch HACKED`"]}'
-    );
-    notifier.notify(options);
+    notifier.notify({ title: 'title', message: 'body', icon: 'icon-string' });
   });
 
   it('should remove extra options that are not supported by notify-send', function(done) {

diff --git a/test/terminal-notifier.js b/test/terminal-notifier.js
@@ -211,7 +211,7 @@ describe('terminal-notifier', function() {
         '-message',
         '"body \\"message\\""',
         '-actions',
-        'foo,bar,baz "foo" bar',
+        '"foo","bar","baz \\"foo\\" bar"',
         '-json',
         '"true"'
       ];