GNU bug report logs - #8427
[SECURITY] sql.el -- comint process passwords are leaked to ps(1) listing

Previous Next

Full log

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

From: Andrew Hyatt <ahyatt <at> gmail.com>
To: Michael Mauger <mmauger <at> protonmail.com>
Cc: "8427 <at> debbugs.gnu.org" <8427 <at> debbugs.gnu.org>,
 Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#8427: [SECURITY] sql.el -- comint process passwords are
 leaked to ps(1) listing
Date: Sun, 15 Dec 2019 23:59:28 -0500

[Message part 1 (text/plain, inline)]

Any input on this?  I believe this fixes the issue, and would prefer to
revise this while I still remember the details.  I'm happy to submit this
as well.

On Mon, Nov 11, 2019 at 12:31 AM Andrew Hyatt <ahyatt <at> gmail.com> wrote:

>
> I've simplified an implementation along the lines you suggest, and
> tested it via ert. I'm attaching the latest version of the patch.
> Please let me know what you think.
>
>
>
> Michael Mauger <mmauger <at> protonmail.com> writes:
>
> > On Saturday, November 2, 2019 1:10 AM, Andrew Hyatt <ahyatt <at> gmail.com>
> wrote:
> >
> >> Michael Mauger mmauger <at> protonmail.com writes:
> >>
> >> > On Sunday, October 20, 2019 8:56 PM, Andrew Hyatt ahyatt <at> gmail.com
> wrote:
> >> >
> >>
> >> Your advice is good, but following it led me to some complexity I can't
> >> seem to get away from. Perhaps you have some insight, so let me explain.
> >> The issue is that, yes, I can not advise the comint function. However,
> >> if I supply my own function, then I have to remove the
> >> comint-watch-for-password-prompt, supply my own function, then restore
> >> it when the user has entered their password (so it can handle subsequent
> >> password entries). This juggling of the normal
> >> comint-watch-for-password-prompt method, plus the fact that we basically
> >> have to reimplement part of it, gives me pause - I think it's probably
> >> too hacky a solution.
> >>
> >> There's a few ways out. We could introduce a variable used in
> >> sql-product-alist that tells SQL not to prompt for a password because
> >> the db will just get it via the comint password function. That would
> >> probably work well, but it wouldn't store the sql-password at all, that
> >> variable would be unused. Maybe that's OK, maybe not - I don't have a
> >> good sense for it.
> >>
> >> Or, we could make this auto-password-supplying per-buffer a part of
> >> comint itself. That would widen the scope of the fix, but it would
> >> probably be the best of both functionality and simplicity.
> >>
> >> What do you think?
> >>
> >
> > I totally understand the complexity, but I don't think it has too be too
> > complicated to address.
> >
> > First the sql.el only solution: If the sql-comint function decides to
> pass
> > the password via stdin then it can set a buffer-local flag indicating
> this
> > and then replace `coming-watch-for-password-prompt' on the
> > `comint-output-filter-functions' list with the sql version of the
> function.
> > The sql password function would be something along the lines of:
> >
> >     ;; TOTALLY NOT TESTED
> >     (defun sql-watch-for-password-prompt (string)
> >       "blah blah ;)"
> >       (if sql-will-prompt-for-password
> >           ;; (based on comint-watch-for-password-prompt)  vvv
> >           (when (let ((case-fold-search t))
> >                   (string-match (or (sql-get-product-feature sql-product
> 'password-prompt-regexp string)
> >                                     comint-password-prompt-regexp)))
> >             (when (string-match "^[ \n\r\t\v\f\b\a]+" string)
> >               (setq string (replace-match "" t t string)))
> >             (let ((comint--prompt-recursion-depth (1+
> comint--prompt-recursion-depth)))
> >               (if (> comint--prompt-recursion-depth 10)
> >                   (message "Password prompt recursion too deep")
> >                 ;;; ^^^
> >                 ;;; automagically provide the password
> >                 (let ((proc (get-buffer-process (current-buffer))))
> >                   (when proc
> >                     (funcall comint-input-sender proc sql-password))))))
> >         ;; Back to default behavior
> >         (comint-watch-for-password-prompt string))
> >       ;; Make sure we don't supply again
> >       (setq-local sql-will-prompt-password nil))
> >
> > That should get you close without too much difficulty. Of course, it
> requires a
> > that a password-prompt-regexp feature is defined for the sql product and
> that the
> > sql-comint function defines a buffer-local flag
> `sql-will-prompt-for-password' in
> > it is deferring to stdin.
> >
> > The other solution would involve modifying comint to call a hook if set
> to supply
> > a password or nil. This would probably be a simpler change but may get
> more
> > broader attention. When the hook function is not set or returns nil then
> do the
> > default behavior of calling `comint-send-invisible' otherwise just send
> the password
> >
> > There are some edge cases here, but this hopefully helps. Also,
> obviously, test cases
> > are needed given that if this breaks, we break the sql interactive world!
> >
> > --
> > MICHAEL <at> MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer
>

[Message part 2 (text/html, inline)]

Previous Next

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