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

Compatibility issue with UltraVNC viewer #108

Closed
wqweto opened this issue Jan 20, 2024 · 3 comments
Closed

Compatibility issue with UltraVNC viewer #108

wqweto opened this issue Jan 20, 2024 · 3 comments

Comments

@wqweto
Copy link

wqweto commented Jan 20, 2024

I recently stumbled on the fact that in RFB_CLIENT_TO_SERVER_SET_ENCODINGS message UltraVNC client sometimes sends more that 32 (pseudo-)encodings.

In this case there is a problem in on_client_set_encodings function in server.c because there is this check

	size_t n_encodings = MIN(MAX_ENCODINGS, ntohs(msg->n_encodings));

. . . and the incoming msg->n_encodings count is limited up to 32 encodings.

Probably MAX_ENCODINGS limit should be checked on accepted encodings in client->encodings array as it is sized to MAX_ENCODINGS + 1 elements which will allow incoming number of encodings to be above MAX_ENCODINGS limit as this is harmless.

The issue is exacerbated because on_client_set_encodings returns sizeof(*msg) + 4 * n_encodings but n_encodings is trimmed above at 32 so the effect is that next message is started from inside incompletely parsed RFB_CLIENT_TO_SERVER_SET_ENCODINGS data and is usually invalid type so client connection gets closed.

@any1
Copy link
Owner

any1 commented Jan 22, 2024

Probably MAX_ENCODINGS limit should be checked on accepted encodings in client->encodings array as it is sized to MAX_ENCODINGS + 1 elements which will allow incoming number of encodings to be above MAX_ENCODINGS limit as this is harmless.

Yep, that's a good idea.

@any1 any1 closed this as completed in 65fc23c Jan 24, 2024
@any1
Copy link
Owner

any1 commented Jan 24, 2024

@wqweto, can you confirm that the issue is fixed?

@wqweto
Copy link
Author

wqweto commented Jan 25, 2024

Yes, just tested the fix is working ok with 33 encodings sent from UltraVNC client.

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

No branches or pull requests

2 participants