Comment 2 for bug 1808989

Revision history for this message
Pavel Cheremushkin (pn.cheremushkin) wrote :

In particular, following 4 issues are related to to tightvnc viewer:

```
1. global buffer overflow in corre.c
    In `vnc_unixsrc/vncviewer/corre.c` inside the `HandleCoRREBPP` function global buffer overflow occurs due to the lack of size check.
    `buffer` is defined in rfbproto.c:96 as ```char buffer[640*480];```. Inside `HandleCoRREBPP` function data is being read to the buffer `ReadFromRFBServer(buffer, hdr.nSubrects * (4 + (BPP / 8))` where `hdr.nSubrects` is 32-bit unsigned integer controlled by remote user.

2. heap buffer overflow in rfbServerCutText handler
    Heap buffer overflow in `rfbServerCutText` handler inside `HandleRFBServerMessage` happens due to the malloc argument unsigned integer overflow on line rfbproto.c:1220. Suppose msg.sct.length equals 0xffffffff, then `malloc(msg.sct.length+1);` = `malloc(0);` will allocate small heap chunk of size 0x10. But `msg.sct.length` = 0xffffffff bytes may be read in this chunk on line 1222 (`ReadFromRFBServer(serverCutText, msg.sct.length)`).

3. heap buffer overflow in InitialiseRFBConnection function
    Heap buffer overflow `InitialiseRFBConnection` function happens due to the malloc argument unsigned integer overflow on line rfbproto.c:307. Because of the integer overflow `malloc` function will allocate small heap chunk of size 0x10 and 0xffffffff bytes will be read into the chunk by ReadFromRFBServer function.

4. null-ptr dereference in `zlib.c`
    Because malloc result is not checked after allocation on line zlib.c:56 null pointer dereference is possible if malloc argument is too big and malloc fill fail to allocate memory Allocation of raw buffer : `raw_buffer = (char*) malloc( raw_buffer_size );`, next usage of raw_buffer is on line 68
```

P.S. As stated in the same thread of the mailing list by Solar Designer tightvnc (as well as libvnc) suffers from user completely controlling size of allocation, which may lead to resource exhaustion, and also LibVNC fix by casting to (uint64_t) seems to be insufficient, because malloc() has size_t argument and issue will remain on 32-bit platforms. So proper allocation limiting is required to completely fix this issue.