From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#30190: 27.0.50; term run in line mode shows user passwords Date: Wed, 18 Jul 2018 20:02:12 -0400 Message-ID: <87zhyo6qkb.fsf@gmail.com> References: <87r2qjh0fs.fsf@gmail.com> <87mv17nwe4.fsf@users.sourceforge.net> <87efm259s5.fsf@gmail.com> <83vafe9f16.fsf@gnu.org> <87wozfkt9t.fsf@gmail.com> <87o9kiejd4.fsf@gmail.com> <83606q6xr7.fsf@gnu.org> <873718qpme.fsf@gmail.com> <87in6erte5.fsf@gmail.com> <83efh1s9s3.fsf@gnu.org> <87602drqan.fsf@gmail.com> <8336xgsvt3.fsf@gnu.org> <878t6892pv.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1531958471 8131 195.159.176.226 (19 Jul 2018 00:01:11 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 19 Jul 2018 00:01:11 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) Cc: 30190@debbugs.gnu.org, Tino Calancha To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Jul 19 02:01:07 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ffwNV-00022T-GO for geb-bug-gnu-emacs@m.gmane.org; Thu, 19 Jul 2018 02:01:05 +0200 Original-Received: from localhost ([::1]:38738 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ffwPc-0004vb-CW for geb-bug-gnu-emacs@m.gmane.org; Wed, 18 Jul 2018 20:03:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ffwPS-0004vN-GY for bug-gnu-emacs@gnu.org; Wed, 18 Jul 2018 20:03:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ffwPO-0005HU-HQ for bug-gnu-emacs@gnu.org; Wed, 18 Jul 2018 20:03:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:42285) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ffwPO-0005HF-BM for bug-gnu-emacs@gnu.org; Wed, 18 Jul 2018 20:03:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ffwPN-00077b-O0 for bug-gnu-emacs@gnu.org; Wed, 18 Jul 2018 20:03:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 19 Jul 2018 00:03:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30190 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed security Original-Received: via spool by 30190-submit@debbugs.gnu.org id=B30190.153195854227321 (code B ref 30190); Thu, 19 Jul 2018 00:03:01 +0000 Original-Received: (at 30190) by debbugs.gnu.org; 19 Jul 2018 00:02:22 +0000 Original-Received: from localhost ([127.0.0.1]:47303 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ffwOk-00076b-4s for submit@debbugs.gnu.org; Wed, 18 Jul 2018 20:02:22 -0400 Original-Received: from mail-it0-f52.google.com ([209.85.214.52]:52270) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ffwOi-00076O-60 for 30190@debbugs.gnu.org; Wed, 18 Jul 2018 20:02:20 -0400 Original-Received: by mail-it0-f52.google.com with SMTP id p4-v6so6681545itf.2 for <30190@debbugs.gnu.org>; Wed, 18 Jul 2018 17:02:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=9jMwt5Mr/bu2k/Gboa88Vo5QEOrG8CU8l8id+uWS2pA=; b=lXo61Py7jOg5XO4rzTj5cyCkmM+YqRlWWKsI1DcroAN8+kfARibcjqlSD7Sz66ZXF0 i942RloKyPurUicmadoE4Qz+AQIRpOlA1OUNx9cN3VMRRagbjfFHTRszxOPiknuliGbF PR0nLUchhqFbPdmqDcVV9f/D4JFHGKkvTBdl1vPzkibFD8aM7Z6Ok/+bK5wfXeOqEfAX aGjZf/9nEsF6VOg3ZAxo8d4sJYyy3t96XQhYXzLSQO0AKia/GReDduMErxWGYvbSJ7Sm enG1BNwbgUA71uttCiT3fMmA5E4a3Ed4hUGut+YvOfQXzHkyzZfnpP3TrV5UlJ/1ZTFH xcTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=9jMwt5Mr/bu2k/Gboa88Vo5QEOrG8CU8l8id+uWS2pA=; b=M7vtWD314LHByxA//pdrSn6cjrbpopWkE2oAEpG/H7JcC45fgYPu37IPTpXNXWocPG VGMmArKQc8h09cdj/CeRVW3mtKplfRzJHWOWSmhqVm6VBV8kykIsIwBIQHICl7jjPeNO yD6ofS7iampPIV+kkXleF8nWpjWqN9egzvfHy422TbPZsOlN/1JJ/9wibz4FCHWfqWf1 KSOiuaJUMYlqT2khpRodYFCx6yJB4eLi88epGD3+X8E2AzKujXRzdxrAXShtP4l1xn8f /qmav8MUCdML8nqJDZd3dzlsarq64U7v1qnEOouPWZJIMYWuzKEzZWyBNS2zYxXD/Cw6 baEQ== X-Gm-Message-State: AOUpUlHhMrNigYknQUKM99gMCQk9osIN67L6imJqtc4PDAbwIn0E5NNt SpEsM+lWR9X5o+AOvLOzvzQ= X-Google-Smtp-Source: AAOMgpd13JjX/mjvRO1WAI4u5CxJjzLh4AtJ1A27eP9ThFETbF5LWippSfvDcxtOTCncJ3LeXIZQ7w== X-Received: by 2002:a24:f945:: with SMTP id l66-v6mr3177038ith.6.1531958534648; Wed, 18 Jul 2018 17:02:14 -0700 (PDT) Original-Received: from zebian (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.googlemail.com with ESMTPSA id w191-v6sm2172305ita.33.2018.07.18.17.02.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 18 Jul 2018 17:02:13 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Wed, 18 Jul 2018 10:24:14 -0400") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:148650 Archived-At: --=-=-= Content-Type: text/plain Stefan Monnier writes: > Couldn't help send you some nitpicks, tho, Thanks for reviewing :) >> @@ -2288,7 +2289,8 @@ term-send-invisible >> \\[view-lossage]." >> (interactive "P") ; Defeat snooping via C-x esc >> (when (not (stringp str)) >> - (setq str (term-read-noecho "Non-echoed text: " t))) >> + (let ((read-hide-char ?*)) >> + (setq str (read-passwd "Non-echoed text: ")))) >> (when (not proc) >> (setq proc (get-buffer-process (current-buffer)))) >> (if (not proc) (error "Current buffer has no process") > > Why do we need to bind `read-hide-char` here? > More specifically, shouldn't `read-passwd` do that for us (hence if it > doesn't yet, then the right patch is to add this let-binding to > `read-passwd`)? Tino mentioned "*" being more visible than ".", but poking at this a bit more, I notice that term-read-noecho uses "*", so I guess that was the original motivation. I've dropped the read-hide-char binding, I think it probably doesn't matter much either way. Another thing I noticed is that read-passwd doesn't have the view-lossage leak that term-read-noecho has, so I've removed that note from the docstring. >> +(defun term-watch-for-password-prompt (string) >> + "Prompt in the minibuffer for password and send without echoing. >> +This function uses `term-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'." > I don't see any reason to document in the docstring what internal > mechanism is used Makes sense, I've trimmed the docstring. >> @@ -3152,6 +3165,9 @@ term-emulate-terminal >> (term-handle-deferred-scroll)) >> >> (set-marker (process-mark proc) (point)) >> + (when (stringp decoded-substring) >> + (term-watch-for-password-prompt (prog1 decoded-substring >> + (setq decoded-substring nil)))) > > I suggest you add a comment explaining why we set decoded-substring to nil. Ah, I carefully wrote a comment explaining why I did that, and then I realized it was wrong. There's not actually any need for it (I had got a bit mixed up and thought we might loop around and prompt twice, but this call is already after the loop). --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v4-0001-Prevent-line-mode-term-from-showing-user-password.patch Content-Description: patch >From 429082a5e14abefb503d39390044b92cd2328462 Mon Sep 17 00:00:00 2001 From: Tino Calancha Date: Thu, 15 Feb 2018 09:09:50 +0900 Subject: [PATCH v4] Prevent line-mode term 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. Make buffers in line term-mode to hide passwords too (Bug#30190). * lisp/term.el (term-send-invisible): Prefer the more robust `read-passwd' instead of `term-read-noecho'. (term-watch-for-password-prompt): New function. (term-emulate-terminal): Call it each time we receive non-escape sequence output. Co-authored-by: Noam Postavsky --- lisp/term.el | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lisp/term.el b/lisp/term.el index b7f5b0e7f2..adbc0b0d88 100644 --- a/lisp/term.el +++ b/lisp/term.el @@ -347,6 +347,7 @@ term-protocol-version (eval-when-compile (require 'cl-lib)) (require 'ring) (require 'ehelp) +(require 'comint) ; Password regexp. (declare-function ring-empty-p "ring" (ring)) (declare-function ring-ref "ring" (ring index)) @@ -2283,12 +2284,10 @@ term-read-noecho (defun term-send-invisible (str &optional proc) "Read a string without echoing. Then send it to the process running in the current buffer. A new-line -is additionally sent. String is not saved on term input history list. -Security bug: your string can still be temporarily recovered with -\\[view-lossage]." +is additionally sent. String is not saved on term input history list." (interactive "P") ; Defeat snooping via C-x esc (when (not (stringp str)) - (setq str (term-read-noecho "Non-echoed text: " t))) + (setq str (read-passwd "Non-echoed text: "))) (when (not proc) (setq proc (get-buffer-process (current-buffer)))) (if (not proc) (error "Current buffer has no process") @@ -2297,6 +2296,16 @@ term-send-invisible (term-send-string proc str) (term-send-string proc "\n"))) +;; TODO: Maybe combine this with `comint-watch-for-password-prompt'. +(defun term-watch-for-password-prompt (string) + "Prompt in the minibuffer for password and send without echoing. +Checks if STRING contains a password prompt as defined by +`comint-password-prompt-regexp'." + (when (term-in-line-mode) + (when (let ((case-fold-search t)) + (string-match comint-password-prompt-regexp string)) + (term-send-invisible (read-passwd string))))) + ;;; Low-level process communication @@ -3152,6 +3161,8 @@ term-emulate-terminal (term-handle-deferred-scroll)) (set-marker (process-mark proc) (point)) + (when (stringp decoded-substring) + (term-watch-for-password-prompt decoded-substring)) (when save-point (goto-char save-point) (set-marker save-point nil)) -- 2.11.0 --=-=-=--