Crashes UltraVNC server that has MSLogonII enabled

[bk138]

If you'd like to put out an incentive for fixing this bug, you can do so at https://issuehunt.io/r/bk138/multivnc?tab=idle

Is your bug report about the Desktop Multivnc or the Mobile MultiVNC?

• Desktop (wxWidgets) or Mobile (Android): Android

Which MultiVNC version are you using?

2.0.9

Which server are you connecting to?

• VNC server vendor: UltraVNC
• VNC server version: 1.3.8.1
• OS version: Windows 10
• OS language: German

Describe the bug

Crashes UltraVNC server that has MSLogonII enabled

To Reproduce

1. Enable MSLogon II in UltraVNC Server admin panel as per libvncclient: implement UltraVNC's MSLogonII authentication scheme LibVNC/libvncserver#480 (comment)
2. Connect with Android MultiVNC 2.0.9
3. enter username and password
4. server crashes

Expected Behavior

server does not crash ;-)

Screenshots

For the Desktop Version (please complete the following information):

• OS and version:
• Xorg version used:
• Wayland version used:

For the Mobile Version (please complete the following information):

• Android version: 12
• Installed from Play Store, F-Droid, Amazon Appstore: any

Additional context

• Probably needs a debug build of the server to see what's going on...
• Another libvncclient-based client worked and still works: libvncclient: implement UltraVNC's MSLogonII authentication scheme LibVNC/libvncserver#480 (comment)
• Maybe wireshark on the server side might be useful to diff the communication flows.

[bk138]

MSLogon w/ MultiVNC 2.0.9

server terminates afterwards 💣 ...

but also, in another run:

MSLogon w/ SDLvncviewer built from MultiVNC's libvncserver submodule

works OK ✔️

[bk138]

LibVNC/libvncserver#493 is related...

[pdlan]

I also found this bug of UltraVNC when implementing MSLogonII for noVNC and TigerVNC. It seems if the username/password is long enough or is not null terminalted, the UltraVNC server will crash. If this happens, the encryption algorithm is likely to be incorrect so the server gets the wrong decrypteed plaintext (which may be long or not null terminated).

[bk138]

@pdlan were you able to pinpoint the exact cause so we can find some kinda fix/workaround? Is it:

• credentials length > x -> crash
• username and or password not 0-terminated -> crash
  ?

[pdlan]

I just tested it. When the username length is > 179 the server crashes. The password length doesn't matter.

[bk138]

I had the problem with "usual" usernames as well. What about the 0-termination?

[pdlan]

The case without null termination should be similar to long usernames because the length can be as long as 256 (if there isn't a zero by accident). If your username is very short (e.g. crashes even if the length is 8), I guess it's mostly likely to be because you don't handle DES-CBC correctly, i.e., all blocks but the first one are corrupted.

[bk138]

The case without null termination should be similar to long usernames because the length can be as long as 256 (if there isn't a zero by accident). If your username is very short (e.g. crashes even if the length is 8), I guess it's mostly likely to be because you don't handle DES-CBC correctly, i.e., all blocks but the first one are corrupted.

Yeah true.

[gujjwal00]

So, I ran UltraVNC under Visual Studio debugger and found that it is throwing an exception during DH key exchange. The exception is not handled, leading to this crash.

Exception source: https://github.com/ultravnc/UltraVNC/blob/1586ec05577d7b82eb19cac5746cf02d6823df5b/rfb/dh.cpp#L136
Called from: https://github.com/ultravnc/UltraVNC/blob/1586ec05577d7b82eb19cac5746cf02d6823df5b/winvnc/winvnc/vncclient.cpp#L1718

After looking around a bit, it seems that UltraVNC limits DH key to 31bits, and when LibVNCClient sends 64bit public key, it throws an exception.

BTW, AVNC with wolfSSL has same issue, so it doesn't seem to related to OpenSSL specifically.

I can't explain why it works in some cases for you guys. For me, It always crashes, even for 4-5 character username.

UltraVNC version: 1.4.0.6 from GitHub

[pdlan]

Oh, it seems that you guys might find a different bug from what I found. The long username bug occurs even with UltraVNC client + server.

[pdlan]

BTW I don't think the length limit is a issue because the UltraVNC server always sends a prime < maxNum, so no matter how big the client's private key is, the public key sent by it should always be < prime < maxNum as long as the implementation is correct.

[gujjwal00]

The issue seems to be incorrect handling of buffers used for holding DH keys crypto_openssl.c.

dh_generate_keypair() & dh_compute_shared_key() are given 8-byte buffers to fill with DH keys. But if a key is smaller, e.g. here it is only 4-byte long, it will be stored in starting bytes of the buffer. Later, when that buffer is treated as full 8-byte key, it results in incorrect calculations.

Now, LibVNCClient already uses BN_bn2binpad which automatically pads the buffer with zeros, but it is not available in older OpenSSL/LibreSSL version. LibVNCClient will fall back to BN_bn2bin which doesn't add zeros. This is probably why SDLvncviewer is working, as it might have been compiled with OpenSSL 1.1.1.

If I manually pad the buffer with this quick patch, I can connect successfully.

[bk138]

If I manually pad the buffer with this quick patch, I can connect successfully.

Great work @gujjwal00! (as always) Can you whip up a PR for libvncclient?

[gujjwal00]

Sure 👍