rofi-rbw icon indicating copy to clipboard operation
rofi-rbw copied to clipboard

WIP: Change the way `xclip` handles clearing

Open jacob-horton opened this issue 3 years ago • 3 comments

Only clear the xclip clipboard if nothing has been copied since the password. This prevents something else being cleared from the clipboard if it was copied between copying the password and it being cleared.

This does, however, mean that the password is stored in memory (in the previously_copied variable) until it is cleared from the clipboard. Also, if --clear-after is not used, it will stay in the previously_copied variable until overwritten by the next copy. This is potentially a security risk, so I am unsure whether this feature is worth the reduction in security.

jacob-horton avatar Jul 09 '22 00:07 jacob-horton

I'm not too worried about the security implications - we already store the password in memory at run time, so this could already be exploited. But if you have that kind of access already, you can probably do worse things already.

My main question is if this is actually a problem: The user asked rofi-rbw to clear the clipboard after some time, probably some seconds. Is it likely that they are copying something else in the meantime? Or are we trying to solve a problem that doesn't really exist?

fdw avatar Jul 10 '22 09:07 fdw

I agree that it is not a common issue. I have had this happen to me only a handful of times over several years of using a password manager, but I'm not sure how often other people encounter this, if at all. That being said, it can be confusing if it does happen, although since --clear-after has to be explicitly specified, the user would be more aware than if clearing happened by default.

Since xsel is the default (at least on my system) and does not have this 'issue', I do not think it's a problem that many people will encounter, but it could be nice to have the consistency between xsel and xclip.

I am not sure whether this is actually an issue, but that's my thoughts on it, hope that helps you make your decision :slightly_smiling_face:

jacob-horton avatar Jul 11 '22 15:07 jacob-horton

Hey, sorry this took so long. I wanted to revisit it with a fresh mind, but it fell off my radar 🙁

However, it doesn't add a lot of complexity and it might help at least one person (you 😉), so I'll leave a short code review and then we can merge it.

Thanks for your patience!

fdw avatar Aug 28 '22 16:08 fdw

I have just found that the same "issue" occurs with xsel, I don't know if there has been a change, or if I missed it before. Would you like me to do the same changes for that? And if so, should I add it to the base class? I don't use Wayland, so I do not know if wtype has this issue

jacob-horton avatar Oct 09 '22 16:10 jacob-horton

Feel free to add it to xsel, too 🙂

However, I wouldn't want to do it in the base class, as not every clipboarder necessarily has that problem.

fdw avatar Oct 13 '22 16:10 fdw

I've now added it for xsel :slightly_smiling_face:

jacob-horton avatar Oct 15 '22 15:10 jacob-horton

Thank you! 🙂

fdw avatar Oct 15 '22 19:10 fdw

@jacob-horton Can you please have a look at my last question and maybe rebase again?

fdw avatar Nov 27 '22 12:11 fdw

I had some problems merging it directly, so I rebased, squashed and merged it manually. Hope this is okay for you 🙂

fdw avatar Dec 01 '22 14:12 fdw