any1 / neatvnc

A liberally licensed VNC server library with a clean interface
ISC License
126 stars 31 forks source link

Compatibility issue with UltraVNC viewer #108

Closed wqweto closed 10 months ago

wqweto commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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

wqweto commented 10 months ago

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