Summary

There exists an integer overflow in HandleCursorShape RFB event handler in libvncclient, which is the client implementation for LibVNC included with libvncserver.

This issue allows a malicious VNC server to trigger a remote heap overflow in a connecting VNC client, which may lead to Remote Code Excution (RCE). This issue was addressed in libvncclient 0.9.13.

NOTE: this issue was addressed in the LibVNC pre-release branch on Nov 17, 2019, prior to GHSL’s discovery of the same issue in a release version (0.9.11) as shipped in a mainline Linux distribution. In coordination with the maintainer the GHSL decided to still fully triage this issue with CVE assignment so that the various Linux distributions shipping this library would be aware of the included security fix and update accordingly.

Product

libvncserver/libvncclient

Tested Versions

Details: Potential integer overflow in HandleCursorShape RFB event handling

All source snippets are based on the 0.9.11 release of libvncserver.

libvncclient/cursor.c
...
rfbBool HandleCursorShape(rfbClient* client,int xhot, int yhot, int width, int height, uint32_t enc)
{
  int bytesPerPixel;
  size_t bytesPerRow, bytesMaskData;
  rfbXCursorColors rgb;
  uint32_t colors[2];
  char *buf;
  uint8_t *ptr;
  int x, y, b;

  bytesPerPixel = client->format.bitsPerPixel / 8;
  bytesPerRow = (width + 7) / 8;
  bytesMaskData = bytesPerRow * height;

  if (width * height == 0)
    return TRUE;

  /* Allocate memory for pixel data and temporary mask data. */
  if(client->rcSource)
    free(client->rcSource);

[1]
  client->rcSource = malloc(width * height * bytesPerPixel);
  if (client->rcSource == NULL)
    return FALSE;

  buf = malloc(bytesMaskData);
  if (buf == NULL) {
    free(client->rcSource);
    client->rcSource = NULL;
    return FALSE;
  }

  /* Read and decode cursor pixel data, depending on the encoding type. */

  if (enc == rfbEncodingXCursor) {
    /* Read and convert background and foreground colors. */
    if (!ReadFromRFBServer(client, (char *)&rgb, sz_rfbXCursorColors)) {
      free(client->rcSource);
      client->rcSource = NULL;
      free(buf);
      return FALSE;
    }
    colors[0] = RGB24_TO_PIXEL(32, rgb.backRed, rgb.backGreen, rgb.backBlue);
    colors[1] = RGB24_TO_PIXEL(32, rgb.foreRed, rgb.foreGreen, rgb.foreBlue);

    /* Read 1bpp pixel data into a temporary buffer. */
    if (!ReadFromRFBServer(client, buf, bytesMaskData)) {
      free(client->rcSource);
      client->rcSource = NULL;
      free(buf);
      return FALSE;
    }

    /* Convert 1bpp data to byte-wide color indices. */
    ptr = client->rcSource;
    for (y = 0; y < height; y++) {
      for (x = 0; x < width / 8; x++) {
	for (b = 7; b >= 0; b--) {
	  *ptr = buf[y * bytesPerRow + x] >> b & 1;
	  ptr += bytesPerPixel;
	}
      }
      for (b = 7; b > 7 - width % 8; b--) {
	*ptr = buf[y * bytesPerRow + x] >> b & 1;
	ptr += bytesPerPixel;
      }
    }

    /* Convert indices into the actual pixel values. */
    switch (bytesPerPixel) {
    case 1:
      for (x = 0; x < width * height; x++)
	client->rcSource[x] = (uint8_t)colors[client->rcSource[x]];
      break;
    case 2:
      for (x = 0; x < width * height; x++)
	((uint16_t *)client->rcSource)[x] = (uint16_t)colors[client->rcSource[x * 2]];
      break;
    case 4:
      for (x = 0; x < width * height; x++)
	((uint32_t *)client->rcSource)[x] = colors[client->rcSource[x * 4]];
      break;
    }

  } else {			/* enc == rfbEncodingRichCursor */

    if (!ReadFromRFBServer(client, (char *)client->rcSource, width * height * bytesPerPixel)) {
      free(client->rcSource);
      client->rcSource = NULL;
      free(buf);
      return FALSE;
    }

  }

  /* Read and decode mask data. */

  if (!ReadFromRFBServer(client, buf, bytesMaskData)) {
    free(client->rcSource);
    client->rcSource = NULL;
    free(buf);
    return FALSE;
  }

  client->rcMask = malloc(width * height);
  if (client->rcMask == NULL) {
    free(client->rcSource);
    client->rcSource = NULL;
    free(buf);
    return FALSE;
  }

  ptr = client->rcMask;
  for (y = 0; y < height; y++) {
    for (x = 0; x < width / 8; x++) {
      for (b = 7; b >= 0; b--) {
	*ptr++ = buf[y * bytesPerRow + x] >> b & 1;
      }
    }
    for (b = 7; b > 7 - width % 8; b--) {
      *ptr++ = buf[y * bytesPerRow + x] >> b & 1;
    }
  }

  if (client->GotCursorShape != NULL) {
     client->GotCursorShape(client, xhot, yhot, width, height, bytesPerPixel);
  }

  free(buf);

  return TRUE;
}

When handling rfbFramebufferUpdate RFB messages, the libvnclient code will check whether rect.encoding is set to rfbEncodingXcursor or rfbEncodingRichcursor. If this is the case, it will then pass remote (server side) controlled 16bit integer values as parameters into HandleCursorShape. This includes the xhot, yhot, width, height and enc parameters.

At [1] we observe a 32bit integer arithmetic expression that involves the width, height and bytesPerPixel integers. A malicious VNC server can control up to 16bits of the width and height integers, and it may set the bytesPerPixel integer to 1,2 or 4 based on its control over the “Bits Per Pixel” value of the session (8bits, 24bits, or 32bits, divided by 8).

This level of control over the allocation size integer arithmetic expression allows a malicious VNC server to induce an integer overflow and subsequently under-allocate the intended client->rcSource memory region.

The subsequent population of this memory region is based on the width and height parameters, which will in turn result in potentially exploitable memory tresspasses.

Impact

It is possible to craft this vulnerability into a remote heap overflow which, under the right circumstances, may result in remote code execution. However, due to the specifics of the integer overwrap which results in a fairly limited range allocation control vs overflow length, we do not consider this likely.

CVE

Coordinated Disclosure Timeline

Resources

Credit

This issue was discovered and reported by the GitHub Security Lab.

Contact

You can contact the GHSL team at securitylab@github.com, please include GHSL-2020-064 in any communication regarding this issue.