Re: LibVNCServer rfbserver.c: rfbProcessClientNormalMessage() case rfbClientCutText doesn't sanitize msg.cct.length

[Solar Designer <solar () openwall com>]

On Sun, Feb 18, 2018 at 07:09:45PM +0100, Solar Designer wrote:
> vcSetXCutTextProc() came from LibVNCServer-0.9.9/vncterm/VNConsole.c, so its
> shortcomings also need to be reported to LibVNCServer upstream.

> vncterm exists as a separate repo,
> so I might report its issues in there: https://github.com/LibVNC/vncterm

Reported vncterm: VNConsole.c: vcSetXCutTextProc() integer overflow and
unchecked malloc():

https://github.com/LibVNC/vncterm/issues/6

vncterm's implementation of the callback is:

void vcSetXCutTextProc(char* str,int len, struct _rfbClientRec* cl)
{
  vncConsolePtr c=(vncConsolePtr)cl->screen->screenData;

  if(c->selection) free(c->selection);
  c->selection=(char*)malloc(len+1);
  memcpy(c->selection,str,len);
  c->selection[len]=0;
}

Besides the conversion to signed int during the call (a LibVNCServer API
issue), there's also len+1 in the implementation, which may cause an
integer overflow resulting in e.g. malloc(0) (which succeeds) followed
by memcpy(..., ..., -1) (which writes beyond the allocated memory).  And
there's no check for malloc() possibly returning NULL.

I did not request CVE ID(s) for this, and I don't intend to do so.  If
you need to, please feel free to track the vncterm vcSetXCutTextProc()
issues above as OVE-20180218-0002.

Alexander