#1603 KeePass 2.35 is corrupting clipboard on Linux

[David Lechner]

I noticed this with 2.35 from Julian Taylors PPA, but I have also tested with the official portable download. I'm running Ubuntu 16.04 with mono 4.8.0 from the xamarin ppa.

I have confirmed that the bug is present in 2.35, but not in 2.34. I have also confirmed the same behavior in an Ubuntu 16.10 VM with the official Ubuntu mono (4.2.1).

The actual bug:

When copying and pasting large amounts of text (1000+ lines), if KeePass is running, it will corrupt the contents of the clipboard. usually, this involves appending a combination of parts of the copied text along with non-printable characters to the end of the copied text. Some text editors can handle this (like gedit) where you can see the bad data and others (like Sublime Text 3) will ignore the data, so it seems as if copy and paste is not working.

• To repduce, open a large text file in gedit (1000+ lines).
• Open KeePass 3.35 (no need to open a database)
• Select all and copy the text in gedit
• Open a new tab or second instance of gedit and paste the text
• Move to the end of the pasted text and you should see extra garbage

For example, when I just tried this, I get this at the end of the file:

MACHINE_ENDude <linux/inQ
#include <linq of any kind, �

The actual end of the file is just

MACHINE_END

I notice that in the v3.35 changelog (History.txt) says:

• Added workaround for Mono clipboard bug

Do you think this could be the source of the problem? If so, could you point out where in the code I should be looking?

[Dominik Reichl]

Indeed this is most likely caused by the new clipboard workaround. I cannot reproduce the problem on Kubuntu 16.10 though.

Since KeePass 2.35, there's a background thread that periodically checks whether the clipboard contents have changed. When this is the case, it copies the new data to the clipboard again using 'xsel' (pasting then is possible in other applications).

You can find the workaround in the KeePassLib/Utility/MonoWorkarounds.cs file, method FixClipThread. The corresponding development discussion is here:
https://sourceforge.net/p/keepass/bugs/1530/

My guess is that retrieving or sending clipboard contents from/to 'xsel' fails for some reason. I don't really see how this could occur though; I'll need to setup an Ubuntu system for properly testing this.

Best regards,
Dominik

[David Lechner]

I read up on the other bug and found that I did not have xdotool installed. Installing it seems to fix the problem. So, I think the solution here is to make sure KeePass only implements this workaround if xdotool is installed - or find a way to not depend on xdotool.

[Dominik Reichl]

xdotool is only used for determining whether the workaround is required. If it's not installed, KeePass assumes that the workaround is required. If it's installed, KeePass uses it to check whether KeePass or Remmina are in the foreground and only applies the workaround in these cases.

So, installing xdotool may limit the problem to KeePass and Remmina, but it's not a fix for the actual problem.

I'm currently downloading Ubuntu 16.10 and will try it.

Best regards,
Dominik

[David Lechner]

I have found that

string str = NativeLib.RunConsoleApp(strXSel, "--output --clipboard");

is causing the problem for me. Apparently calling xsel --output --clipboard a bunch of times results in corrupting the clipboard - probably a bug in xsel.

But in general, I don't like the looks of this workaround.

        private static void FixClipThread()
        {
            try
            {
#if !KeePassUAP
                const string strXSel = "xsel";
                const AppRunFlags rfW = AppRunFlags.WaitForExit;

                string strLast = null;
                while(true)
                {
                    string str = NativeLib.RunConsoleApp(strXSel,
                        "--output --clipboard");
                    if(str == null) return; // 'xsel' not installed

                    if(str != strLast)
                    {
                        if(NeedClipboardWorkaround())
                            NativeLib.RunConsoleApp(strXSel,
                                "--input --clipboard", str, rfW);

                        strLast = str;
                    }

                    Thread.Sleep(250);
                }
#endif
            }
            catch(ThreadAbortException)
            {
                try { Thread.ResetAbort(); }
                catch(Exception) { Debug.Assert(false); }
            }
            catch(Exception) { Debug.Assert(false); }
            finally { m_thFixClip = null; }
        }

Here is my thinking. This loop is running 4 times every second. Each loop, it reads the entirety of the clipboard and creates a new string object. If I have a large amount of data on the clip board, this means lots of memory usage and lots garbage collection for all of those strings.

I'll dig around some more and see if I can come up with a better workaround.

[Dominik Reichl]

This indeed looks like an xsel bug, independent of Mono. I wrote a small script for testing:

for i in 0 1 2 3 4 5 6 7 8 9
do
    xsel --output --clipboard > Clip.tmp
    md5sum -b Clip.tmp
    cat Clip.tmp | xsel --input --clipboard
done

For small clipboard contents, this works flawlessly, but larger ones usually get corrupted after some iterations (randomly). When commenting out the last line of the loop, everything works fine, too.

As clipboard contents are provided via IPC, xsel forks itself to be able to provide the new clipboard contents after an --input call; immediately calling xsel with --output then apparently can fail (maybe the fork isn't fully ready yet?).

In order to avoid this, I've now changed the arguments for the --input call to: --input --clipboard --nodetach. The --nodetach option prevents xsel from forking; the original xsel instance continues to run until the clipboard contents change. As KeePass waits for the termination of xsel, this blocks our clipboard thread for a while, but that doesn't matter.

Here's the latest development snapshot for testing (including the modified source code file):
http://keepass.info/filepool/KeePass_170125b.zip
(should also work without xdotool).

On the performance: I doubt reading the clipboard periodically and storing the last contents is a problem; I'd be very surprised if Mono's garbage collector couldn't handle this.

Thanks and best regards,
Dominik

[David Lechner]

Using a blocking version of xsel is much better. The snapshot addresses my previous concenrns, however, I have identified another problem.

If xdotool is not installed, the the workaround is always executed. This causes all clipboard contents to be converted to plain text. For example, if I am writing a document in Libre Office Writer and I try to copy and paste formatted text, it will lose the formatting and paste plain text.

So, at the very least, I think this workaroud should not run if xdotool is not installed.

However, even if xdotool is installed and the workaround is limited to KeePass and Remmina, this will cause the same problem when using Remmina. If you copy formatted text in any application in Remmina, KeePass will strip the formatting and covert it to plain text.

So, my recommendataion would be to make this workaround optional and disabled by default.

[Anonymous]

I noticed the same on ArchLinux KDE. No xdotool, xsel installed. KP v2.35.
What interesting, clipboard operations within KP like copying triggers into notes fields are weird and extra text editor like Kate is required to successfully perform such copying. What Mono bugs do require xsel with its such a harsh and intrusive polling usage?

[Dominik Reichl]

Ok, I've changed this now. If XDoTool is not installed, the workaround is not applied anymore.

In the case of Remmina and KeePass, I think it's very likely that users will copy data between KeePass and a Remmina window, thus the workaround will continue to be applied here by default (if XDoTool is installed). Users who prefer to copy complex data within Remmina can turn off the workaround using the -wa-disable:1530 command line option.

Here's the latest development snapshot for testing (including the modified source code file):
http://keepass.info/filepool/KeePass_170226.zip

Thanks and best regards,
Dominik

[Dominik Reichl]

With a slightly changed XSel invocation that is more fault-tolerant:
http://keepass.info/filepool/KeePass_170226b.zip

Thanks and best regards,
Dominik