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

[Michael Mauger via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

On Saturday, November 2, 2019 1:10 AM, Andrew Hyatt <ahyatt@gmail.com> wrote:

> Michael Mauger mmauger@protonmail.com writes:
>
> > On Sunday, October 20, 2019 8:56 PM, Andrew Hyatt ahyatt@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@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer