Merge pull request #41 from newsoft/master

Fixing 2 security issues
pull/1/head
dscho 10 years ago
commit 7e9ce73b5d

@ -1259,13 +1259,20 @@ typedef struct {
#define RFB_FILE_ATTRIBUTE_TEMPORARY 0x100 #define RFB_FILE_ATTRIBUTE_TEMPORARY 0x100
#define RFB_FILE_ATTRIBUTE_COMPRESSED 0x800 #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; int x;
char *home=NULL; char *home=NULL;
FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE); 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: */ /* C: */
if (path[0]=='C' && path[1]==':') if (path[0]=='C' && path[1]==':')
strcpy(unixPath, &path[2]); strcpy(unixPath, &path[2]);
@ -1274,6 +1281,10 @@ rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, char *path, char *unixPath)
home = getenv("HOME"); home = getenv("HOME");
if (home!=NULL) if (home!=NULL)
{ {
/* Re-check buffer size */
if ((strlen(path) + strlen(home) + 1) >= unixPathMaxLen)
return FALSE;
strcpy(unixPath, home); strcpy(unixPath, home);
strcat(unixPath,"/"); strcat(unixPath,"/");
strcat(unixPath, path); strcat(unixPath, path);
@ -1318,7 +1329,8 @@ rfbBool rfbSendDirContent(rfbClientPtr cl, int length, char *buffer)
FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE); FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE);
/* Client thinks we are Winblows */ /* 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); 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 */ /* 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; if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
/* The client requests a File */ /* 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); 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); 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 */ /* If the file exists... We can send a rfbFileChecksums back to the client before we send an rfbFileAcceptHeader */
/* TODO: Delta Transfer */ /* 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; if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
switch (contentParam) { switch (contentParam) {
case rfbCDirCreate: /* Client requests the creation of a directory */ 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); retval = mkdir(filename1, 0755);
if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCDirCreate(\"%s\"->\"%s\") %s\n", buffer, filename1, (retval==-1?"Failed":"Success")); 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); if (buffer!=NULL) free(buffer);
return retval; return retval;
case rfbCFileDelete: /* Client requests the deletion of a file */ 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 (stat(filename1,&statbuf)==0)
{ {
if (S_ISDIR(statbuf.st_mode)) 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) */ /* Split into 2 filenames ('*' is a seperator) */
*p = '\0'; *p = '\0';
rfbFilenameTranslate2UNIX(cl, buffer, filename1); if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
rfbFilenameTranslate2UNIX(cl, p+1, filename2); goto fail;
if (!rfbFilenameTranslate2UNIX(cl, p+1, filename2, sizeof(filename2)))
goto fail;
retval = rename(filename1,filename2); 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")); 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! */ /* NOTE: don't forget to free(buffer) if you return early! */
if (buffer!=NULL) free(buffer); if (buffer!=NULL) free(buffer);
return TRUE; return TRUE;
fail:
if (buffer!=NULL) free(buffer);
return FALSE;
} }
/* /*

@ -66,6 +66,12 @@
(double) ((int) (x)) : (double) ((int) (x) + 1) ) (double) ((int) (x)) : (double) ((int) (x) + 1) )
#define FLOOR(x) ( (double) ((int) (x)) ) #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) int ScaleX(rfbScreenInfoPtr from, rfbScreenInfoPtr to, int x)
{ {
@ -281,14 +287,29 @@ rfbScreenInfoPtr rfbScaledScreenAllocate(rfbClientPtr cl, int width, int height)
ptr = malloc(sizeof(rfbScreenInfo)); ptr = malloc(sizeof(rfbScreenInfo));
if (ptr!=NULL) if (ptr!=NULL)
{ {
int allocSize;
/* copy *everything* (we don't use most of it, but just in case) */ /* copy *everything* (we don't use most of it, but just in case) */
memcpy(ptr, cl->screen, sizeof(rfbScreenInfo)); 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->width = width;
ptr->height = height; ptr->height = height;
ptr->paddedWidthInBytes = (ptr->bitsPerPixel/8)*ptr->width; ptr->paddedWidthInBytes = (ptr->bitsPerPixel/8)*ptr->width;
/* Need to by multiples of 4 for Sparc systems */ /* 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! */ /* Reset the reference count to 0! */
ptr->scaledScreenRefCount = 0; ptr->scaledScreenRefCount = 0;

Loading…
Cancel
Save