diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c index f64ea4e..21f9eff 100644 --- a/libvncserver/rfbserver.c +++ b/libvncserver/rfbserver.c @@ -1259,13 +1259,20 @@ typedef struct { #define RFB_FILE_ATTRIBUTE_TEMPORARY 0x100 #define RFB_FILE_ATTRIBUTE_COMPRESSED 0x800 -rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, char *path, char *unixPath) +rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, /* in */ char *path, /* out */ char *unixPath, size_t unixPathMaxLen) { int x; char *home=NULL; FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE); + /* + * Do not use strncpy() - truncating the file name would probably have undesirable side effects + * Instead check if destination buffer is big enough + */ + if (strlen(path) >= unixPathMaxLen) + return FALSE; + /* C: */ if (path[0]=='C' && path[1]==':') strcpy(unixPath, &path[2]); @@ -1274,6 +1281,10 @@ rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, char *path, char *unixPath) home = getenv("HOME"); if (home!=NULL) { + /* Re-check buffer size */ + if ((strlen(path) + strlen(home) + 1) >= unixPathMaxLen) + return FALSE; + strcpy(unixPath, home); strcat(unixPath,"/"); strcat(unixPath, path); @@ -1318,7 +1329,8 @@ rfbBool rfbSendDirContent(rfbClientPtr cl, int length, char *buffer) FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE); /* Client thinks we are Winblows */ - rfbFilenameTranslate2UNIX(cl, buffer, path); + if (!rfbFilenameTranslate2UNIX(cl, buffer, path, sizeof(path))) + return FALSE; if (DB) rfbLog("rfbProcessFileTransfer() rfbDirContentRequest: rfbRDirContent: \"%s\"->\"%s\"\n",buffer, path); @@ -1659,7 +1671,8 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con /* add some space to the end of the buffer as we will be adding a timespec to it */ if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE; /* The client requests a File */ - rfbFilenameTranslate2UNIX(cl, buffer, filename1); + if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1))) + goto fail; cl->fileTransfer.fd=open(filename1, O_RDONLY, 0744); /* @@ -1774,7 +1787,8 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con } sizeHtmp = Swap32IfLE(sizeHtmp); - rfbFilenameTranslate2UNIX(cl, buffer, filename1); + if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1))) + goto fail; /* If the file exists... We can send a rfbFileChecksums back to the client before we send an rfbFileAcceptHeader */ /* TODO: Delta Transfer */ @@ -1903,7 +1917,8 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE; switch (contentParam) { case rfbCDirCreate: /* Client requests the creation of a directory */ - rfbFilenameTranslate2UNIX(cl, buffer, filename1); + if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1))) + goto fail; retval = mkdir(filename1, 0755); if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCDirCreate(\"%s\"->\"%s\") %s\n", buffer, filename1, (retval==-1?"Failed":"Success")); /* @@ -1912,7 +1927,8 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con if (buffer!=NULL) free(buffer); return retval; case rfbCFileDelete: /* Client requests the deletion of a file */ - rfbFilenameTranslate2UNIX(cl, buffer, filename1); + if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1))) + goto fail; if (stat(filename1,&statbuf)==0) { if (S_ISDIR(statbuf.st_mode)) @@ -1930,8 +1946,10 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con { /* Split into 2 filenames ('*' is a seperator) */ *p = '\0'; - rfbFilenameTranslate2UNIX(cl, buffer, filename1); - rfbFilenameTranslate2UNIX(cl, p+1, filename2); + if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1))) + goto fail; + if (!rfbFilenameTranslate2UNIX(cl, p+1, filename2, sizeof(filename2))) + goto fail; retval = rename(filename1,filename2); if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCFileRename(\"%s\"->\"%s\" -->> \"%s\"->\"%s\") %s\n", buffer, filename1, p+1, filename2, (retval==-1?"Failed":"Success")); /* @@ -1951,6 +1969,10 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con /* NOTE: don't forget to free(buffer) if you return early! */ if (buffer!=NULL) free(buffer); return TRUE; + +fail: + if (buffer!=NULL) free(buffer); + return FALSE; } /* diff --git a/libvncserver/scale.c b/libvncserver/scale.c index 6f91391..d5b3f8b 100644 --- a/libvncserver/scale.c +++ b/libvncserver/scale.c @@ -66,6 +66,12 @@ (double) ((int) (x)) : (double) ((int) (x) + 1) ) #define FLOOR(x) ( (double) ((int) (x)) ) +static inline int pad4(int value) +{ + int remainder = value & 3; + if (!remainder) return value; + return value + 4 - remainder; +} int ScaleX(rfbScreenInfoPtr from, rfbScreenInfoPtr to, int x) { @@ -281,14 +287,29 @@ rfbScreenInfoPtr rfbScaledScreenAllocate(rfbClientPtr cl, int width, int height) ptr = malloc(sizeof(rfbScreenInfo)); if (ptr!=NULL) { + int allocSize; + /* copy *everything* (we don't use most of it, but just in case) */ memcpy(ptr, cl->screen, sizeof(rfbScreenInfo)); + + /* SECURITY: make sure that no integer overflow will occur afterwards. + * Note: this is defensive coding, as the check should have already been + * performed during initial, non-scaled screen setup. + */ + allocSize = pad4(width * (ptr->bitsPerPixel/8)); /* per protocol, width<2**16 and bpp<256 */ + if (height == 0 || allocSize >= SIZE_MAX / height) + { + free(ptr); + return NULL; /* malloc() will allocate an incorrect buffer size - early abort */ + } + + /* Resume copy everything */ ptr->width = width; ptr->height = height; ptr->paddedWidthInBytes = (ptr->bitsPerPixel/8)*ptr->width; /* Need to by multiples of 4 for Sparc systems */ - ptr->paddedWidthInBytes += (ptr->paddedWidthInBytes % 4); + ptr->paddedWidthInBytes = pad4(ptr->paddedWidthInBytes); /* Reset the reference count to 0! */ ptr->scaledScreenRefCount = 0;