GNU bug report logs - #17127
`call-process' circumvents password concealment w/ `read-passwd'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17127 in the body.
You can then email your comments to 17127 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nathan Trapuzzano <nbtrap <at> nbtrap.com>
To: bug-gnu-emacs <at> gnu.org
Subject: `call-process' circumvents password concealment w/ `read-passwd'
Date: Thu, 27 Mar 2014 20:32:55 -0400

To reproduce with emacs -nw -q on 24.3 and trunk:

  M-: (global-set-key
        (kbd "C-c C-c")
        (lambda ()
          (interactive)
          (call-process "echo" nil t nil "-n" "foobar")))

  M-: (read-passwd "Password: ")

  C-c C-c

"foobar" is printed in the minibuffer rather than "......", whereas,
e.g., yanking from the kill ring print dots.

Message #8 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Nathan Trapuzzano <nbtrap <at> nbtrap.com>
Cc: 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Thu, 27 Mar 2014 22:04:32 -0400

> To reproduce with emacs -nw -q on 24.3 and trunk:

>   M-: (global-set-key
>         (kbd "C-c C-c")
>         (lambda ()
>           (interactive)
>           (call-process "echo" nil t nil "-n" "foobar")))

>   M-: (read-passwd "Password: ")

>   C-c C-c

This looks fairly contrived.  How did you stumble upon this problem?


        Stefan

Message #11 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Nathan Trapuzzano <nbtrap <at> nbtrap.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Thu, 27 Mar 2014 22:39:41 -0400

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> This looks fairly contrived.  How did you stumble upon this problem?

Copy/pasting passwords from console password manager to emacs running on
terminal emulator in X.  The built-in copy/paste functionaly for the X
clipboard only works (AFAIK) with graphical emacs, so I use my own
commands to make it work on a terminal.  Here's the one that made me
catch it:

(defun paste-from-X-clipboard ()
  "Insert the X clipboard contents at point."
  (interactive)
  (call-process "xclip" nil t nil "-selection" "clipboard" "-o"))

I use that to paste passwords when, e.g., finding remote files via
ssh/TRAMP.

Message #14 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Nathan Trapuzzano <nbtrap <at> nbtrap.com>
Cc: 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Sun, 29 Sep 2019 16:35:22 +0200

Nathan Trapuzzano <nbtrap <at> nbtrap.com> writes:

> To reproduce with emacs -nw -q on 24.3 and trunk:
>
>   M-: (global-set-key
>         (kbd "C-c C-c")
>         (lambda ()
>           (interactive)
>           (call-process "echo" nil t nil "-n" "foobar")))
>
>   M-: (read-passwd "Password: ")
>
>   C-c C-c
>
> "foobar" is printed in the minibuffer rather than "......", whereas,
> e.g., yanking from the kill ring print dots.

The following patch fixes this, I think, by using post-command-hook
instead of after-change-functions.

It seems to work for me -- does anybody see a problem with doing it this
way?

diff --git a/lisp/subr.el b/lisp/subr.el
index 45b99a82d2..9e4553dcbb 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2426,6 +2426,12 @@ read-passwd-map
     map)
   "Keymap used while reading passwords.")
 
+(defun read-password--hide-password ()
+  (let ((beg (minibuffer-prompt-end)))
+             (dotimes (i (1+ (- (buffer-size) beg)))
+               (put-text-property (+ i beg) (+ 1 i beg)
+                                  'display (string (or read-hide-char ?*))))))
+
 (defun read-passwd (prompt &optional confirm default)
   "Read a password, prompting with PROMPT, and return it.
 If optional CONFIRM is non-nil, read the password twice to make sure.
@@ -2450,15 +2456,7 @@ read-passwd
               (message "Password not repeated accurately; please start over")
               (sit-for 1))))
         success)
-    (let ((hide-chars-fun
-           (lambda (beg end _len)
-             (clear-this-command-keys)
-             (setq beg (min end (max (minibuffer-prompt-end)
-                                     beg)))
-             (dotimes (i (- end beg))
-               (put-text-property (+ i beg) (+ 1 i beg)
-                                  'display (string (or read-hide-char ?*))))))
-          minibuf)
+    (let (minibuf)
       (minibuffer-with-setup-hook
           (lambda ()
             (setq minibuf (current-buffer))
@@ -2469,7 +2467,7 @@ read-passwd
             (use-local-map read-passwd-map)
             (setq-local inhibit-modification-hooks nil) ;bug#15501.
 	    (setq-local show-paren-mode nil)		;bug#16091.
-            (add-hook 'after-change-functions hide-chars-fun nil 'local))
+            (add-hook 'post-command-hook 'read-password--hide-password nil t))
         (unwind-protect
             (let ((enable-recursive-minibuffers t)
 		  (read-hide-char (or read-hide-char ?*)))
@@ -2479,7 +2477,8 @@ read-passwd
               ;; Not sure why but it seems that there might be cases where the
               ;; minibuffer is not always properly reset later on, so undo
               ;; whatever we've done here (bug#11392).
-              (remove-hook 'after-change-functions hide-chars-fun 'local)
+              (remove-hook 'after-change-functions 'read-password--hide-password
+                           'local)
               (kill-local-variable 'post-self-insert-hook)
               ;; And of course, don't keep the sensitive data around.
               (erase-buffer))))))))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #17 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Nathan Trapuzzano <nbtrap <at> nbtrap.com>
Cc: 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Sun, 13 Oct 2019 05:16:03 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>> "foobar" is printed in the minibuffer rather than "......", whereas,
>> e.g., yanking from the kill ring print dots.
>
> The following patch fixes this, I think, by using post-command-hook
> instead of after-change-functions.
>
> It seems to work for me -- does anybody see a problem with doing it this
> way?

There were no comments in two weeks, so I've now applied the patch.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #24 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Nathan Trapuzzano <nbtrap <at> nbtrap.com>, 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Wed, 23 Oct 2019 18:01:45 -0400

> The following patch fixes this, I think, by using post-command-hook
> instead of after-change-functions.

Actually, in theory after-change-functions should catch all cases
whereas post-command-hook might miss some (i.e. chars inserted not
while running a command, e.g. from a process filter).

So while your new code probably works fine in practice (and is a good
workaround for now) , I think the original code is "more correct" and we
should try and figure out why it didn't work: how come
after-change-functions is not run (or not correctly) by call-process?


        Stefan

Message #27 received at 17127 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Nathan Trapuzzano <nbtrap <at> nbtrap.com>, 17127 <at> debbugs.gnu.org
Subject: Re: bug#17127: `call-process' circumvents password concealment w/
 `read-passwd'
Date: Thu, 24 Oct 2019 13:49:21 +0200

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> The following patch fixes this, I think, by using post-command-hook
>> instead of after-change-functions.
>
> Actually, in theory after-change-functions should catch all cases
> whereas post-command-hook might miss some (i.e. chars inserted not
> while running a command, e.g. from a process filter).
>
> So while your new code probably works fine in practice (and is a good
> workaround for now) , I think the original code is "more correct" and we
> should try and figure out why it didn't work: how come
> after-change-functions is not run (or not correctly) by call-process?

Yeah, that's a good point.  Data inserted by call-process definitely
changes the buffer, so after-change-functions should be run.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.