Fix argument injection in lilypond-invoke-editor, CVE-2017-17523.

This is a cherry-pick of upstream's fix, see
https://sourceforge.net/p/testlilyissues/issues/5243/

Closes: #884136

diff --git a/debian/patches/Issue-5243-1-editor-scm-Add-shell-quote-argument-function.diff b/debian/patches/Issue-5243-1-editor-scm-Add-shell-quote-argument-function.diff
new file mode 100644 (file)
index 0000000..324b3d3
--- /dev/null
+++ b/debian/patches/Issue-5243-1-editor-scm-Add-shell-quote-argument-function.diff
@@ -0,0 +1,116 @@
+From: David Kastrup <dak@gnu.org>
+Date: Tue, 28 Nov 2017 11:18:07 +0000 (+0100)
+Subject: Issue 5243/1: (editor scm): Add shell-quote-argument function
+X-Git-Url: http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commitdiff_plain;h=807f5eb8cd631133da3be6897e3e8fa7202e089d
+
+Issue 5243/1: (editor scm): Add shell-quote-argument function
+
+This is mostly stolen from Emacs.
+---
+
+diff --git a/scm/editor.scm b/scm/editor.scm
+index 8a34e79..9406055 100644
+--- a/scm/editor.scm
++++ b/scm/editor.scm
+@@ -40,6 +40,100 @@
+         (else
+          "emacs"))))
+ 
++;; A bunch of stuff stolen from Emacs
++
++(define (w32-using-nt)
++  "Return non-nil if running on a Windows NT descendant.
++That includes all Windows systems except for 9X/Me."
++  (getenv "SystemRoot"))
++
++(define (w32-shell-name)
++  "Return the name of the shell being used."
++  (or (getenv "SHELL")
++      (and (w32-using-nt) "cmd.exe")
++      "command.com"))
++
++(define w32-system-shells '("cmd" "cmd.exe" "command" "command.com"
++                            "4nt" "4nt.exe" "4dos" "4dos.exe"
++                            "tcc" "tcc.exe" "ndos" "ndos.exe"))
++
++(define (w32-system-shell-p shell-name)
++  (and shell-name
++       (member (string-downcase
++                (basename shell-name))
++             w32-system-shells)))
++
++(define (w32-shell-dos-semantics)
++  "Return non-nil if the interactive shell being used expects MS-DOS shell semantics."
++  (or (w32-system-shell-p (w32-shell-name))
++      (and (member (string-downcase (basename (w32-shell-name)))
++                 '("cmdproxy" "cmdproxy.exe"))
++         (w32-system-shell-p (getenv "COMSPEC")))))
++
++(define-public (shell-quote-argument argument)
++  "Quote ARGUMENT for passing as argument to an inferior shell.
++
++This function is designed to work with the syntax of your system's
++standard shell, and might produce incorrect results with unusual shells.
++See Info node `(elisp)Security Considerations'."
++  (cond
++   ((and (eq? PLATFORM 'windows) (w32-shell-dos-semantics))
++
++    ;; First, quote argument so that CommandLineToArgvW will
++    ;; understand it.  See
++    ;; http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx
++    ;; After we perform that level of quoting, escape shell
++    ;; metacharacters so that cmd won't mangle our argument.  If the
++    ;; argument contains no double quote characters, we can just
++    ;; surround it with double quotes.  Otherwise, we need to prefix
++    ;; each shell metacharacter with a caret.
++
++    (set! argument
++          ;; escape backslashes at end of string
++          (regexp-substitute/global
++           #f
++           "(\\\\+)$"
++           ;; escape backslashes and quotes in string body
++           (regexp-substitute/global
++            #f
++            "(\\\\*)\""
++            argument
++            'pre 1 1 "\\\"" 'post)
++           'pre 1 1 'post))
++
++    (if (string-match "[%!\"]" argument)
++        (string-append
++         "^\""
++         (regexp-substitute/global
++          #f
++          "[%!()\"<>&|^]"
++          argument
++          'pre "^" 0 'post)
++         "^\"")
++        (string-append "\"" argument "\"")))
++
++   (else
++    (if (string-null? argument)
++        "''"
++        ;; Quote everything except POSIX filename characters.
++        ;; This should be safe enough even for really weird shells.
++        (regexp-substitute/global
++         #f
++         "\n"
++         (regexp-substitute/global
++          #f
++;;;       "[^-0-9a-zA-Z_./\n]" Negative ranges are too dangerous since
++;;;       their UTF-8 implications aren't clear: we don't want
++;;;       characters outside the ASCII range quoted since it is not
++;;;       clear whether we need to quote bytes or characters.  So we just
++;;;       invert the above regexp pattern for Posix characters manually.
++          "[\x01-\x09\x0b-,:-@[-^{-\x7f]"
++          argument
++          'pre "\\" 0 'post)
++         'pre  "'\n'" 'post)))
++   ))
++
++
+ (define editor-command-template-alist
+   '(("emacs" .  "emacsclient --no-wait +%(line)s:%(column)s %(file)s || (emacs +%(line)s:%(column)s %(file)s&)")
+     ("gvim" . "gvim --remote +:%(line)s:norm%(column)s %(file)s")
+

diff --git a/debian/patches/Issue-5243-2-Let-get-editor-use-shell-quote-argument.diff b/debian/patches/Issue-5243-2-Let-get-editor-use-shell-quote-argument.diff
new file mode 100644 (file)
index 0000000..b417259
--- /dev/null
+++ b/debian/patches/Issue-5243-2-Let-get-editor-use-shell-quote-argument.diff
@@ -0,0 +1,24 @@
+From: David Kastrup <dak@gnu.org>
+Date: Tue, 28 Nov 2017 11:19:02 +0000 (+0100)
+Subject: Issue 5243/2: Let get-editor use shell-quote-argument
+X-Git-Url: http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commitdiff_plain;h=39f800a7e5acb7cc5da6424c99fd2690e389495a
+
+Issue 5243/2: Let get-editor use shell-quote-argument
+
+Addresses security concerns.
+---
+
+diff --git a/scm/editor.scm b/scm/editor.scm
+index 9406055..f0132a0 100644
+--- a/scm/editor.scm
++++ b/scm/editor.scm
+@@ -169,7 +169,7 @@ See Info node `(elisp)Security Considerations'."
+   (let* ((editor (get-editor))
+          (template (get-command-template editor-command-template-alist editor))
+          (command
+-          (re-sub "%\\(file\\)s" (format #f "~S" file-name)
++          (re-sub "%\\(file\\)s" (shell-quote-argument file-name)
+                   (re-sub "%\\(line\\)s" (format #f "~a" line)
+                           (re-sub "%\\(char\\)s" (format #f "~a" char)
+                                   (re-sub
+

diff --git a/debian/patches/Issue-5243-3-More-conservative-parsing-of-textedit-URIs.diff b/debian/patches/Issue-5243-3-More-conservative-parsing-of-textedit-URIs.diff
new file mode 100644 (file)
index 0000000..325f5b7
--- /dev/null
+++ b/debian/patches/Issue-5243-3-More-conservative-parsing-of-textedit-URIs.diff
@@ -0,0 +1,22 @@
+From: David Kastrup <dak@gnu.org>
+Date: Tue, 28 Nov 2017 11:19:30 +0000 (+0100)
+Subject: Issue 5243/3: More conservative parsing of textedit URIs
+X-Git-Url: http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commitdiff_plain;h=aee02594be68a968bb843f87d3264777099e46b4
+
+Issue 5243/3: More conservative parsing of textedit URIs
+---
+
+diff --git a/scripts/lilypond-invoke-editor.scm b/scripts/lilypond-invoke-editor.scm
+index 6658f50..de45663 100644
+--- a/scripts/lilypond-invoke-editor.scm
++++ b/scripts/lilypond-invoke-editor.scm
+@@ -93,7 +93,7 @@ Options:
+   
+   
+ (define (dissect-uri uri)
+-  (let* ((match (string-match "textedit://(.*):([^:]+):([^:]+):(.*)$" uri)))
++  (let* ((match (string-match "textedit://(.*):([0-9]+):([0-9]+):([0-9]*)$" uri)))
+     (if match
+       (list (unquote-uri (match:substring match 1))
+             (match:substring match 2)
+

diff --git a/debian/patches/series b/debian/patches/series
index 8e4644c22faa0fdc1902cb8ef11809b1f32e96b2..a6d307a38e56095ade99cdb0adf4a4f803464fa0 100644 (file)
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -19,3 +19,6 @@ use_debians_help2man
 0100-guile-config-link-static-libguile.a-for-lilypond.patch
 0101-read_relocation_dir-in-lilypond_datadir-too.patch
 Issue-4814-grob.cc-segfaults-with-gcc6
+Issue-5243-1-editor-scm-Add-shell-quote-argument-function.diff
+Issue-5243-2-Let-get-editor-use-shell-quote-argument.diff
+Issue-5243-3-More-conservative-parsing-of-textedit-URIs.diff