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

Fix Extended Clipboard implementation issues #561

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gujjwal00
Copy link
Contributor

  1. According to spec, clipboard text is null terminated and text length includes the null byte. This is how LibVNCServer and other clients encode this. Fixed by 2nd commit 7ff0a53

  2. This is a big one. Many servers (including LibVNCServer) can't properly handle zlib streams created by compress() function of zlib library. When decompressing such streams, inflate() correctly returns Z_STREAM_END, but these servers are only expecting Z_OK. So they end up crashing, while LibVNCserver abruptly disconnects from client.

    This is probably why the original PR Libvncclient clipboard utf8 support #552 was not working. New PR simply suppressed it by adding extra byte, which made inflate() return Z_OK.

Solution:

According to zlib manual (https://zlib.net/manual.htm):

"inflate() returns Z_OK if some progress has been made (more input processed or more output produced),
Z_STREAM_END if the end of the compressed data has been reached and all uncompressed output has been produced"
Text data sent via extended clipboard encoding is null-terminated and this null character is counted in the length field sent to server.

E.g. for "abc", bytes  {'a', 'b', 'c', '\0'} are sent with length = 4.
Many servers can't handle zlib streams created by compress() function
of zlib library. Primary bug is that these servers don't expect inflate()
to return Z_STREAM_END, which is the case for  streams created by
compress().

Affected servers:
LibVNCServer
UltraVNC
TigerVNC (<= 1.10)
@gujjwal00
Copy link
Contributor Author

cc @wuhanck

@wuhanck
Copy link
Contributor

wuhanck commented Mar 6, 2023

it looks ok. it is safer if client dont include \0 in the end.

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 this pull request may close these issues.

3 participants