[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#30190: 27.0.50; term run in line mode shows user passwords
From: |
Noam Postavsky |
Subject: |
bug#30190: 27.0.50; term run in line mode shows user passwords |
Date: |
Wed, 18 Jul 2018 20:02:12 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Stefan Monnier <monnier@IRO.UMontreal.CA> 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).
v4-0001-Prevent-line-mode-term-from-showing-user-password.patch
Description: patch
- bug#30190: 27.0.50; term run in line mode shows user passwords, Noam Postavsky, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Tino Calancha, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Stefan Monnier, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Tino Calancha, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Stefan Monnier, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Tino Calancha, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Stefan Monnier, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Noam Postavsky, 2018/07/18
- bug#30190: 27.0.50; term run in line mode shows user passwords, Stefan Monnier, 2018/07/19
- bug#30190: 27.0.50; term run in line mode shows user passwords, Tino Calancha, 2018/07/20
bug#30190: 27.0.50; term run in line mode shows user passwords,
Noam Postavsky <=
bug#30190: 27.0.50; term run in line mode shows user passwords, Stefan Monnier, 2018/07/23
bug#30190: 27.0.50; term run in line mode shows user passwords, Tino Calancha, 2018/07/23
bug#30190: 27.0.50; term run in line mode shows user passwords, Noam Postavsky, 2018/07/23
bug#30190: 27.0.50; term run in line mode shows user passwords, Eli Zaretskii, 2018/07/23
bug#30190: 27.0.50; term run in line mode shows user passwords, Noam Postavsky, 2018/07/29