Bug#550843: ldapvi: does not handle EDITOR values with arguments - Same for PAGER
retitle 550843 ldapvi: does neither handle $EDITOR nor $PAGER values with arguments kthxbye Hi, I ran into this issue today, too, with $PAGER set to less -s like in this thread on the upstream ML: http://lists.askja.de/pipermail/ldapvi/2011-May/92.html Rhonda wrote on Wed, 14 Oct 2009 11:29:34 +0200: following that approach might raise security related issues with injecting escape sequences, Are you seriously saying that hg, svn and git have security issues because of this? Because they work fine with my setting. Erm, about git (svn): http://osdir.com/ml/git/2009-02/msg02581.html, But that seems only for git-svn and not git itself. With git it works fine since ages (I use less -XF as pager in git, works fine even on git 1.5.x on Debian Lenny), same for mutt with $EDITOR where I use emacsclient -a emacs or even more complex constructs. See http://bugs.debian.org/656657 for a more complete discussion of this topic. (I actually wrote that bug report during research for details for this mail. :-) it doesn't work - and there are also some interesting responses to it. The only interesting ones are IMHO those suggesting to pass the whole string as argument to sh -c: http://osdir.com/ml/git/2009-02/msg02582.html http://osdir.com/ml/git/2009-02/msg02589.html (The link on http://osdir.com/ml/git/2009-02/msg02678.html is broken due to some msg-id-looks-like-email obfuscating techniques, so there maybe more advise, but the link itself doesn't help.) Anyway, yes, I'm seriously saying that it might be troublesome and non-trivial to do it right. IMHO the current state ignores the rule of least surprise (and the issued error messages make that even worse). Commands with whitespace in the path are extremly seldom and setting commands with parameters via environment variables and passing them to sh -c is very common. It is also defined for at least $PAGER in the POSIX specification of mailx[1] and man[2]. [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mailx.html [2] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/man.html In both is declared: PAGER Determine a string representing an output filtering or pagination command for writing the output to the terminal. Any string acceptable as a command_string operand to the sh -c command shall be valid. So at least for $PAGER some parts of POSIX expect the value of the variable to be handled like shell code. I though found no such declaration for $EDITOR or $VISUAL in that document, though. Nevertheless I'd expect those variables to be handled identically. Even though David hasn't yet decided (or at least not yet responded) on the proposed patch(es), I would include them in the Debian package for the sake of the least surprise, working pager and editor options, less confusing error messages and a non-crashing ldapvi in case you try to use a pager with options twice in a row. Regards, Axel -- ,''`. | Axel Beckert a...@debian.org, http://people.debian.org/~abe/ : :' : | Debian Developer, ftp.ch.debian.org Admin `. `' | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE `-| 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#550843: ldapvi: does not handle EDITOR values with arguments
tags 550843 +patch
thanks
Here’s a patch to fix this by launching the editor command via /bin/sh. I
also sent this upstream:
http://lists.askja.de/pipermail/ldapvi/2010-December/84.htmldiff -u ldapvi-1.7/debian/changelog ldapvi-1.7/debian/changelog
--- ldapvi-1.7/debian/changelog
+++ ldapvi-1.7/debian/changelog
@@ -1,3 +1,10 @@
+ldapvi (1.7-8) unstable; urgency=low
+
+ * debian/patches/06_editor-arguments: Handle editor commands with
+arguments. (Closes: #550843)
+
+ -- Anders Kaseorg ande...@mit.edu Tue, 14 Dec 2010 15:33:30 -0500
+
ldapvi (1.7-7) unstable; urgency=low
* Change libreadline5-dev to libreadline-dev (closes: #553795)
diff -u ldapvi-1.7/debian/patches/series ldapvi-1.7/debian/patches/series
--- ldapvi-1.7/debian/patches/series
+++ ldapvi-1.7/debian/patches/series
@@ -5,0 +6 @@
+06_editor-arguments
only in patch2:
unchanged:
--- ldapvi-1.7.orig/debian/patches/06_editor-arguments
+++ ldapvi-1.7/debian/patches/06_editor-arguments
@@ -0,0 +1,44 @@
+From: Anders Kaseorg ande...@mit.edu
+Subject: Handle editor commands with arguments
+
+Previously when the EDITOR environment variable is set to a command
+with arguments, such as ‘emacsclient --alternate-editor emacs’, ldapvi
+would fail to launch the editor:
+
+$ ldapvi
+ 26 entries read
+error (misc.c line 180): No such file or directory
+editor died
+error (ldapvi.c line 83): No such file or directory
+
+Fix this by launching the editor via /bin/sh.
+
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=550843
+Forwarded: http://lists.askja.de/pipermail/ldapvi/2010-December/84.html
+Last-Update: 2010-12-14
+
+---
+ misc.c |6 --
+ 1 files changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/misc.c b/misc.c
+index 3b6896e..c50ff0d 100644
+--- a/misc.c
b/misc.c
+@@ -172,9 +172,11 @@ edit(char *pathname, long line)
+ if (line 0) {
+ char buf[20];
+ snprintf(buf, 20, +%ld, line);
+- execlp(vi, vi, buf, pathname, 0);
++ execlp(/bin/sh, sh, -c, exec $0 \$...@\, vi,
++ buf, pathname, 0);
+ } else
+- execlp(vi, vi, pathname, 0);
++ execlp(/bin/sh, sh, -c, exec $0 \$...@\, vi,
++ pathname, 0);
+ syserr();
+ }
+
+--
+1.7.3.3
+
Bug#550843: ldapvi: does not handle EDITOR values with arguments
Here’s a better patch that also fixes a crash on 64-bit systems (due to
execl(…, 0) instead of execl(…, (char *) NULL)). Also sent upstream:
http://lists.askja.de/pipermail/ldapvi/2010-December/85.html
http://lists.askja.de/pipermail/ldapvi/2010-December/86.html
Andersdiff -u ldapvi-1.7/debian/changelog ldapvi-1.7/debian/changelog
--- ldapvi-1.7/debian/changelog
+++ ldapvi-1.7/debian/changelog
@@ -1,3 +1,12 @@
+ldapvi (1.7-8) unstable; urgency=low
+
+ * debian/patches/06_execlp-null: Cast the trailing NULL on execl()
+calls.
+ * debian/patches/07_editor-arguments: Handle editor commands with
+arguments. (Closes: #550843)
+
+ -- Anders Kaseorg ande...@mit.edu Tue, 14 Dec 2010 17:21:22 -0500
+
ldapvi (1.7-7) unstable; urgency=low
* Change libreadline5-dev to libreadline-dev (closes: #553795)
diff -u ldapvi-1.7/debian/patches/series ldapvi-1.7/debian/patches/series
--- ldapvi-1.7/debian/patches/series
+++ ldapvi-1.7/debian/patches/series
@@ -5,0 +6,2 @@
+06_execlp-null
+07_editor-arguments
only in patch2:
unchanged:
--- ldapvi-1.7.orig/debian/patches/06_execlp-null
+++ ldapvi-1.7/debian/patches/06_execlp-null
@@ -0,0 +1,53 @@
+From: Anders Kaseorg ande...@mit.edu
+Subject: Cast the trailing NULL on execl() calls
+
+From exec(3): “The list of arguments must be terminated by a NULL
+pointer, and, since these are variadic functions, this pointer must be
+cast (char *) NULL.”
+
+This prevents crashes on 64-bit systems, where 0 is a 32-bit integer
+and (char *) NULL is a 64-bit pointer.
+
+Forwarded: http://lists.askja.de/pipermail/ldapvi/2010-December/85.html
+Last-Update: 2010-12-14
+---
+ misc.c |8
+ 1 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/misc.c b/misc.c
+index 3b6896e..e9a0d4c 100644
+--- a/misc.c
b/misc.c
+@@ -172,9 +172,9 @@ edit(char *pathname, long line)
+ if (line 0) {
+ char buf[20];
+ snprintf(buf, 20, +%ld, line);
+- execlp(vi, vi, buf, pathname, 0);
++ execlp(vi, vi, buf, pathname, (char *) NULL);
+ } else
+- execlp(vi, vi, pathname, 0);
++ execlp(vi, vi, pathname, (char *) NULL);
+ syserr();
+ }
+
+@@ -213,7 +213,7 @@ view(char *pathname)
+ case -1:
+ syserr();
+ case 0:
+- execlp(pg, pg, pathname, 0);
++ execlp(pg, pg, pathname, (char *) NULL);
+ syserr();
+ }
+
+@@ -245,7 +245,7 @@ pipeview(int *fd)
+ close(fds[1]);
+ dup2(fds[0], 0);
+ close(fds[0]);
+- execlp(pg, pg, 0);
++ execlp(pg, pg, (char *) NULL);
+ syserr();
+ }
+
+--
+1.7.3.3
+
only in patch2:
unchanged:
--- ldapvi-1.7.orig/debian/patches/07_editor-arguments
+++ ldapvi-1.7/debian/patches/07_editor-arguments
@@ -0,0 +1,43 @@
+From: Anders Kaseorg ande...@mit.edu
+Subject: Handle editor commands with arguments
+
+Previously when the EDITOR environment variable is set to a command
+with arguments, such as ‘emacsclient --alternate-editor emacs’, ldapvi
+would fail to launch the editor:
+
+$ ldapvi
+ 26 entries read
+error (misc.c line 180): No such file or directory
+editor died
+error (ldapvi.c line 83): No such file or directory
+
+Fix this by launching the editor via /bin/sh.
+
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=550843
+Forwarded: http://lists.askja.de/pipermail/ldapvi/2010-December/86.html
+Last-Update: 2010-12-14
+---
+ misc.c |6 --
+ 1 files changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/misc.c b/misc.c
+index e9a0d4c..2e3a1f5 100644
+--- a/misc.c
b/misc.c
+@@ -172,9 +172,11 @@ edit(char *pathname, long line)
+ if (line 0) {
+ char buf[20];
+ snprintf(buf, 20, +%ld, line);
+- execlp(vi, vi, buf, pathname, (char *) NULL);
++ execl(/bin/sh, sh, -c, exec $0 \$...@\, vi,
++buf, pathname, (char *) NULL);
+ } else
+- execlp(vi, vi, pathname, (char *) NULL);
++ execl(/bin/sh, sh, -c, exec $0 \$...@\, vi,
++pathname, (char *) NULL);
+ syserr();
+ }
+
+--
+1.7.3.3
+
Bug#550843: ldapvi: does not handle EDITOR values with arguments
Hi, Quoting Marcus Better (mar...@better.se): The question rather would be: How would you specify a pathname/filename that has spaces in it? By escaping like the shell perhaps? That is what svn documents in the svnbook, yes. following that approach might raise security related issues with injecting escape sequences, Are you seriously saying that hg, svn and git have security issues because of this? Because they work fine with my setting. I wouldn't see it as a security issue, but it's certainly something that needs to be documented well. (If it's shell, then which shell is it? Does it have to be the user's shell?) A quick google survey turns up some IBM tools which explicitly say that EDITOR must be a full path for them, so presumably they even call execv(3) rather than execvp(3), much less system(3). But if prevailing implementation practice among modern tools is to go through the shell, then I'm sympathetic to that request for the sake of compatibility. Do you have a patch for this? d. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#550843: ldapvi: does not handle EDITOR values with arguments
* Marcus Better mar...@better.se [2009-10-14 09:45:30 CEST]: Gerfried Fuchs wrote: EDITOR=emacsclient --alternate-editor emacs The question rather would be: How would you specify a pathname/filename that has spaces in it? By escaping like the shell perhaps? That would break the current way of allowing that and would require complex parsing of the string. following that approach might raise security related issues with injecting escape sequences, Are you seriously saying that hg, svn and git have security issues because of this? Because they work fine with my setting. Erm, about git (svn): http://osdir.com/ml/git/2009-02/msg02581.html, it doesn't work - and there are also some interesting responses to it. Anyway, yes, I'm seriously saying that it might be troublesome and non-trivial to do it right. In fact this EDITOR setting was taken from the Emacs manual, you can find it here under the -a option: Emacs is its own OS so that doesn't really convince me. ;) Besides, it isn't really my call, and if you can come up with a clean patch for it I guess David might consider accepting it. So long! Rhonda -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#550843: ldapvi: does not handle EDITOR values with arguments
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Gerfried Fuchs wrote: EDITOR=emacsclient --alternate-editor emacs The question rather would be: How would you specify a pathname/filename that has spaces in it? By escaping like the shell perhaps? following that approach might raise security related issues with injecting escape sequences, Are you seriously saying that hg, svn and git have security issues because of this? Because they work fine with my setting. In fact this EDITOR setting was taken from the Emacs manual, you can find it here under the -a option: http://www.gnu.org/software/emacs/manual/html_node/emacs/emacsclient-Options.html Cheers, Marcus -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkrVgZUACgkQXjXn6TzcAQmN3QCfWVge3Sm7TjHzRsMEczHPpVX1 WmkAn3kvNRByHh+hMCR07MvKfXmd8RX7 =Wzc0 -END PGP SIGNATURE- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
