From 2cdb290712e54bc6606c6cf3beee605685fef49a Mon Sep 17 00:00:00 2001 From: Mikael Brevik Date: Thu, 11 Mar 2021 20:59:51 +0100 Subject: [PATCH] patch: fixes security issue with non-escaping inputs [GHSL-2020-373] --- lib/utils.js | 8 +++----- test/notify-send.js | 27 ++++++++++++++++++--------- test/terminal-notifier.js | 2 +- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index ed5ce23..1a3b888 100644 --- 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 index aad1364..8b89e24 100644 --- 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 index ea757f4..f14073a 100644 --- 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"' ];