Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo can reveal the password #49

Closed
jthulhu opened this issue Jun 26, 2021 · 4 comments · Fixed by #60
Closed

Undo can reveal the password #49

jthulhu opened this issue Jun 26, 2021 · 4 comments · Fixed by #60

Comments

@jthulhu
Copy link

jthulhu commented Jun 26, 2021

When editing a password, doing undo twice right away reveals the password. This may lead to unwanted password leakage, by pressing undo many times to get to the original state.

@doolio
Copy link
Contributor

doolio commented Nov 28, 2023

Can you provide steps to reproduce this bug?

@jthulhu
Copy link
Author

jthulhu commented Nov 28, 2023

Sure. This bug issue was open long ago, but the bug still exists, although slightly differently than the version I reported: now undoing once is enough to reveal the password. More precisely, to reproduce the bug, follow these steps

  • open the password store in Emacs with M-xpass
  • open any entry, it shows •••••••••••• on the first line
  • undo with M-xundo: this reveals the password.

@doolio
Copy link
Contributor

doolio commented Nov 28, 2023

OK, I've submitted PR #60 which fixes this issue but it may be too extreme as you will not be able to undo other changes in the buffer that you may want to.

@jthulhu
Copy link
Author

jthulhu commented Nov 28, 2023

This seems indeed a bit too extreme. The fact that you can accidentally reveal the password with undo is a problem only for those who actually use undo in these buffers (and that do so despite the mild risk). For this reason, I think that removing the undo altogether is counterproductive, because it "solves" a problem only for those who actually want to undo.

I think the proper solution would be to make the password visibility toggle not count as an undoable action, if possible.

doolio added a commit to doolio/pass that referenced this issue Nov 30, 2023
Any changes to the list in the body of the `let' get clobbered when the
original list is restored.

Fixes: NicolasPetton#49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants