Subject: Re: bug#30190: 27.0.50; term run in line mode shows user passwords

From: Tino Calancha <[email-masked]>
Date: Sun, 04 Feb 2018 01:15:54 +0900

Noam Postavsky <[email-masked]> writes:

> Yes, seems to have been the case for a long time, I can reproduce back
> to 24.3 (oldest Emacs version I have running).
This is a security risk.  I would like to have it fixed ASAP.
Below patch seems to work.  Any feedback would be appreciated.

--8<-----------------------------cut here---------------start------------->8---
commit 089c86c2d8e46fd3f134826881c010409b65d364
Author: tino calancha <[email-masked]>
Date:   Sun Feb 4 01:00:29 2018 +0900

    Prevent term run in line mode from showing user passwords
    
    For buffers whose mode derive from comint-mode, the user
    password is read from the minibuffer and it's hidden.
    A buffer in term-mode and line submode, instead shows
    the passwords.
    This commit forces buffers in line term-mode to hide
    passwords (Bug#30190).
    
    * lisp/term-utils: New file.  Move into it some common code
    shared by comint.el and term.el.
    
    * lisp/comint.el (comint-password-prompt-regexp):
    (send-invisible)
    Move them to the new file.  Prefix them with 'term-utils-'.
    Define aliases to the original names.
    
    * lisp/term.el (term-output-filter-functions): New hook.
    (term-send-input, term-emulate-terminal): Call it.

diff --git a/lisp/comint.el b/lisp/comint.el
index 8dba317099..0e9e13f12e 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -104,6 +104,7 @@
 (require 'ring)
 (require 'ansi-color)
 (require 'regexp-opt)                   ;For regexp-opt-charset.
+(require 'term-utils)
 
 ;; Buffer Local Variables:
 ;;============================================================================
@@ -342,35 +343,6 @@ comint-process-echoes
   :type 'boolean
   :group 'comint)
 
-;; AIX puts the name of the person being su'd to in front of the prompt.
-;; kinit prints a prompt like `Password for [email-masked]: '.
-;; ksu prints a prompt like `Kerberos password for devnull/[email-masked]: '.
-;; ssh-add prints a prompt like `Enter passphrase: '.
-;; plink prints a prompt like `Passphrase for key "[email-masked]": '.
-;; Ubuntu's sudo prompts like `[sudo] password for user:'
-;; Some implementations of passwd use "Password (again)" as the 2nd prompt.
-;; Something called "perforce" uses "Enter password:".
-;; See M-x comint-testsuite--test-comint-password-prompt-regexp.
-(defcustom comint-password-prompt-regexp
-  (concat
-   "\\(^ *\\|"
-   (regexp-opt
-    '("Enter" "enter" "Enter same" "enter same" "Enter the" "enter the"
-      "Old" "old" "New" "new" "'s" "login"
-      "Kerberos" "CVS" "UNIX" " SMB" "LDAP" "PEM" "SUDO"
-      "[sudo]" "Repeat" "Bad" "Retype")
-    t)
-   " +\\)"
-   "\\(?:" (regexp-opt password-word-equivalents) "\\|Response\\)"
-   "\\(?:\\(?:, try\\)? *again\\| (empty for no passphrase)\\| (again)\\)?"
-   ;; "[[:alpha:]]" used to be "for", which fails to match non-English.
-   "\\(?: [[:alpha:]]+ .+\\)?[\\s  ]*[:：៖][\\s  ]*\\'")
-  "Regexp matching prompts for passwords in the inferior process.
-This is used by `comint-watch-for-password-prompt'."
-  :version "27.1"
-  :type 'regexp
-  :group 'comint)
-
 ;; Here are the per-interpreter hooks.
 (defvar comint-get-old-input (function comint-get-old-input-default)
   "Function that returns old text in Comint mode.
@@ -399,7 +371,7 @@ comint-input-filter-functions
 These functions get one argument, a string containing the text to send.")
 
 ;;;###autoload
-(defvar comint-output-filter-functions '(ansi-color-process-output comint-postoutput-scroll-to-bottom comint-watch-for-password-prompt)
+(defvar comint-output-filter-functions '(ansi-color-process-output comint-postoutput-scroll-to-bottom term-utils-watch-for-password-prompt)
   "Functions to call after output is inserted into the buffer.
 One possible function is `comint-postoutput-scroll-to-bottom'.
 These functions get one argument, a string containing the text as originally
@@ -2342,41 +2314,8 @@ comint-read-noecho
 ;; saved -- typically passwords to ftp, telnet, or somesuch.
 ;; Just enter m-x send-invisible and type in your line.
 
-(defun send-invisible (&optional prompt)
-  "Read a string without echoing.
-Then send it to the process running in the current buffer.
-The string is sent using `comint-input-sender'.
-Security bug: your string can still be temporarily recovered with
-\\[view-lossage]; `clear-this-command-keys' can fix that."
-  (interactive "P")			; Defeat snooping via C-x ESC ESC
-  (let ((proc (get-buffer-process (current-buffer)))
-	(prefix
-	 (if (eq (window-buffer) (current-buffer))
-	     ""
-	   (format "(In buffer %s) "
-		   (current-buffer)))))
-    (if proc
-	(let ((str (read-passwd (concat prefix
-					(or prompt "Non-echoed text: ")))))
-	  (if (stringp str)
-	      (progn
-		(comint-snapshot-last-prompt)
-		(funcall comint-input-sender proc str))
-	    (message "Warning: text will be echoed")))
-      (error "Buffer %s has no process" (current-buffer)))))
-
-(defun comint-watch-for-password-prompt (string)
-  "Prompt in the minibuffer for password and send without echoing.
-This function uses `send-invisible' to read and send a password to the buffer's
-process if STRING contains a password prompt defined by
-`comint-password-prompt-regexp'.
-
-This function could be in the list `comint-output-filter-functions'."
-  (when (let ((case-fold-search t))
-	  (string-match comint-password-prompt-regexp string))
-    (when (string-match "^[ \n\r\t\v\f\b\a]+" string)
-      (setq string (replace-match "" t t string)))
-    (send-invisible string)))
+(defalias 'comint-watch-for-password-prompt 'term-utils-watch-for-password-prompt)
+
 
 ;; Low-level process communication
 
diff --git a/lisp/term-utils.el b/lisp/term-utils.el
new file mode 100644
index 0000000000..7edb741bc3
--- /dev/null
+++ b/lisp/term-utils.el
@@ -0,0 +1,118 @@
+;;; term-utils.el --- Common code used by term.el and comint.el  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2018  Tino Calancha
+
+;; Author: Tino Calancha <[email-masked]>
+;; Keywords: processes, terminals
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;
+
+;;; code:
+
+
+;; Retrieve the right defvar value according with the buffer mode.
+(defmacro term-utils--defvar-value (suffix)
+  "Build a defvar with the `major-mode' and SUFFIX, return its value.
+The buffer major mode must be `term-mode' or `comint-mode'."
+  (declare (debug (symbolp)) (indent 0))
+  `(symbol-value
+    (intern-soft
+     (format "%S%S" (if (derived-mode-p 'term-mode) 'term- 'comint-)
+             ',suffix))))
+
+;; AIX puts the name of the person being su'd to in front of the prompt.
+;; kinit prints a prompt like `Password for [email-masked]: '.
+;; ksu prints a prompt like `Kerberos password for devnull/[email-masked]: '.
+;; ssh-add prints a prompt like `Enter passphrase: '.
+;; plink prints a prompt like `Passphrase for key "[email-masked]": '.
+;; Ubuntu's sudo prompts like `[sudo] password for user:'
+;; Some implementations of passwd use "Password (again)" as the 2nd prompt.
+;; Something called "perforce" uses "Enter password:".
+;; See M-x comint-testsuite--test-comint-password-prompt-regexp.
+(defcustom term-utils-password-prompt-regexp
+  (concat
+   "\\(^ *\\|"
+   (regexp-opt
+    '("Enter" "enter" "Enter same" "enter same" "Enter the" "enter the"
+      "Old" "old" "New" "new" "'s" "login"
+      "Kerberos" "CVS" "UNIX" " SMB" "LDAP" "PEM" "SUDO"
+      "[sudo]" "Repeat" "Bad" "Retype")
+    t)
+   " +\\)"
+   "\\(?:" (regexp-opt password-word-equivalents) "\\|Response\\)"
+   "\\(?:\\(?:, try\\)? *again\\| (empty for no passphrase)\\| (again)\\)?"
+   ;; "[[:alpha:]]" used to be "for", which fails to match non-English.
+   "\\(?: [[:alpha:]]+ .+\\)?[\\s  ]*[:：៖][\\s  ]*\\'")
+  "Regexp matching prompts for passwords in the inferior process.
+This is used by `term-utils-watch-for-password-prompt'."
+  :version "27.1"
+  :type 'regexp
+  :group 'term-utils)
+
+(declare-function comint-snapshot-last-prompt "comint")
+
+(defun term-utils-send-invisible (&optional prompt)
+  "Read a string without echoing with PROMPT.
+Then send it to the process running in the current buffer.
+The string is sent using `comint-input-sender' or `term-input-sender'
+depending of the buffer mode.
+Security bug: your string can still be temporarily recovered with
+\\[view-lossage]; `clear-this-command-keys' can fix that."
+  (interactive "P")			; Defeat snooping via C-x ESC ESC
+  (let ((proc (get-buffer-process (current-buffer)))
+	(prefix
+	 (if (eq (window-buffer) (current-buffer))
+	     ""
+	   (format "(In buffer %s) "
+		   (current-buffer)))))
+    (if proc
+	(let ((str (read-passwd (concat prefix
+					(or prompt "Non-echoed text: ")))))
+	  (if (stringp str)
+	      (let ((input-sender-fun (term-utils--defvar-value input-sender)))
+		(if (derived-mode-p 'comint-mode) (comint-snapshot-last-prompt))
+		(funcall input-sender-fun proc str))
+	    (message "Warning: text will be echoed")))
+      (error "Buffer %s has no process" (current-buffer)))))
+
+(defalias 'send-invisible 'term-utils-send-invisible)
+
+(defvar term-raw-map) ; Defined in term.el
+(defun term-utils-term-in-line-mode-p ()
+  "Return non-nil if the buffer is in `term-mode' and line submode."
+  (and (derived-mode-p 'term-mode) (eq (current-local-map) term-raw-map)))
+
+(defun term-utils-watch-for-password-prompt (string)
+  "Prompt in the minibuffer for password and send without echoing.
+This function uses `send-invisible' to read and send a password to the buffer's
+process if STRING contains a password prompt defined by
+`term-utils-password-prompt-regexp'.
+
+This function could be in the lists `comint-output-filter-functions'
+and `term-output-filter-functions'."
+  ;; Do nothing if buffer is in term-mode with line submode
+  (unless (term-utils-term-in-line-mode-p)
+    (when (let ((case-fold-search t))
+	    (string-match term-utils-password-prompt-regexp string))
+      (when (string-match "^[ \n\r\t\v\f\b\a]+" string)
+        (setq string (replace-match "" t t string)))
+      (term-utils-send-invisible string))))
+
+
+(provide 'term-utils)
+;;; term-utils.el ends here
diff --git a/lisp/term.el b/lisp/term.el
index a0313d88da..ed0f1715b2 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -340,6 +340,7 @@ term-protocol-version
 (eval-when-compile (require 'cl-lib))
 (require 'ring)
 (require 'ehelp)
+(require 'term-utils)
 
 (declare-function ring-empty-p "ring" (ring))
 (declare-function ring-ref "ring" (ring index))
@@ -575,6 +576,16 @@ term-input-filter-functions
 
 This variable is buffer-local.")
 
+;;;###autoload
+(defvar term-output-filter-functions '(term-utils-watch-for-password-prompt)
+  "Functions to call after output is inserted into the buffer.
+One possible function is `term-utils-watch-for-password-prompt'.
+These functions get one argument, a string containing the text as originally
+inserted.
+
+You can use `add-hook' to add functions to this list
+either globally or locally.")
+
 (defvar term-input-sender (function term-simple-send)
   "Function to actually send to PROCESS the STRING submitted by user.
 Usually this is just `term-simple-send', but if your mode needs to
@@ -2106,7 +2117,8 @@ term-send-input
 	  (set-marker term-pending-delete-marker pmark-val)
 	  (set-marker (process-mark proc) (point)))
 	(goto-char pmark)
-	(funcall term-input-sender proc input)))))
+	(funcall term-input-sender proc input)
+        (run-hook-with-args 'term-output-filter-functions "")))))
 
 (defun term-get-old-input-default ()
   "Default for `term-get-old-input'.
@@ -3017,6 +3029,8 @@ term-emulate-terminal
 	  (term-handle-deferred-scroll))
 
 	(set-marker (process-mark proc) (point))
+        ;; Run these hooks with point where the user had it.
+        (run-hook-with-args 'term-output-filter-functions str)
 	(when save-point
 	  (goto-char save-point)
 	  (set-marker save-point nil))

--8<-----------------------------cut here---------------end--------------->8---