search

Artawower / blamer.el

A git blame plugin for emacs inspired by VS Code's GitLens plugin
GNU General Public License v3.0
412 stars 15 forks source link

receive password prompt when hovering over line which contains `password` #65

Open gekoke opened 5 months ago

gekoke commented 5 months ago

When hovering a line that is part of a commit and includes the word password (not sure of the exact heuristic), I receive a password prompt.

This only happens when blamer-enable-async-execution-p is set to t.

Can replicate by having a committed line with the following contents:

  - password:
Artawower commented 5 months ago

Thanks for the bug report. Fixed in 0.8.6

alexmurray commented 4 months ago

Hmm I wonder if there is a larger issue here - it feels like the line of text in the buffer is getting interpreted by emacs/async etc in some way - I wonder if it is possible to get command execution via an untrusted buffer through this issue? Whilst the current fix stops async prompting for a password, I fear async may be invoking emacs with a command that is derived from text of the current line.

gekoke commented 4 months ago

I agree - though I'm not proficient enough in Elisp to understand what's going on, I don't feel like the proposed fix of setting (async-prompt-for-password nil) and calling (ignore async-prompt-for-password) is enough to solve the problem in the general case. I don't think this kind of interpreting of the buffer contents should really be possible in the first place.

Artawower commented 4 months ago

I couldn't find any code from the emacs-async package that interprets the result buffer as code. It looks like there is a simple regexp that checks if the output of a command matches the tramp-password-prompt-regexp variable. Of course this is a superficial check, if someone can reproduce potential vulnerabilities I'll check them more thoroughly.

alexmurray commented 4 months ago

ah ok - thanks - yep I agree, the input string is not actually interpreted in anyway - just matched against - so there doesn't appear to be any risk here of command injection etc. Thanks for clarifying this for me and your work on blamer @Artawower.