|
|
41da01 |
commit 06ccdf016154fde8eccb5355613ba04c59127b2e
|
|
|
41da01 |
Author: Nicolas Ruff <nruff@google.com>
|
|
|
41da01 |
Date: Mon Sep 1 14:36:26 2014 +0200
|
|
|
41da01 |
|
|
|
41da01 |
Fix multiple stack-based buffer overflows in file transfer feature
|
|
|
41da01 |
|
|
|
41da01 |
diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c
|
|
|
41da01 |
index df7d74c..445331a 100644
|
|
|
41da01 |
--- a/libvncserver/rfbserver.c
|
|
|
41da01 |
+++ b/libvncserver/rfbserver.c
|
|
|
41da01 |
@@ -1241,21 +1241,35 @@ typedef struct {
|
|
|
41da01 |
#define RFB_FILE_ATTRIBUTE_TEMPORARY 0x100
|
|
|
41da01 |
#define RFB_FILE_ATTRIBUTE_COMPRESSED 0x800
|
|
|
41da01 |
|
|
|
41da01 |
-rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, char *path, char *unixPath)
|
|
|
41da01 |
+rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, /* in */ char *path, /* out */ char *unixPath, size_t unixPathMaxLen )
|
|
|
41da01 |
{
|
|
|
41da01 |
int x;
|
|
|
41da01 |
char *home=NULL;
|
|
|
41da01 |
|
|
|
41da01 |
FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE);
|
|
|
41da01 |
|
|
|
41da01 |
+ /*
|
|
|
41da01 |
+ * Do not use strncpy() - truncating the file name would probably have undesirable side effects
|
|
|
41da01 |
+ * Instead check if destination buffer is big enough
|
|
|
41da01 |
+ */
|
|
|
41da01 |
+
|
|
|
41da01 |
+ if (strlen(path) >= unixPathMaxLen)
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+
|
|
|
41da01 |
/* C: */
|
|
|
41da01 |
if (path[0]=='C' && path[1]==':')
|
|
|
41da01 |
+ {
|
|
|
41da01 |
strcpy(unixPath, &path[2]);
|
|
|
41da01 |
+ }
|
|
|
41da01 |
else
|
|
|
41da01 |
{
|
|
|
41da01 |
home = getenv("HOME");
|
|
|
41da01 |
if (home!=NULL)
|
|
|
41da01 |
{
|
|
|
41da01 |
+ /* Re-check buffer size */
|
|
|
41da01 |
+ if ((strlen(path) + strlen(home) + 1) >= unixPathMaxLen)
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+
|
|
|
41da01 |
strcpy(unixPath, home);
|
|
|
41da01 |
strcat(unixPath,"/");
|
|
|
41da01 |
strcat(unixPath, path);
|
|
|
41da01 |
@@ -1293,7 +1307,8 @@ rfbBool rfbSendDirContent(rfbClientPtr cl, int length, char *buffer)
|
|
|
41da01 |
FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE);
|
|
|
41da01 |
|
|
|
41da01 |
/* Client thinks we are Winblows */
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, path);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, path, sizeof(path)))
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
|
|
|
41da01 |
if (DB) rfbLog("rfbProcessFileTransfer() rfbDirContentRequest: rfbRDirContent: \"%s\"->\"%s\"\n",buffer, path);
|
|
|
41da01 |
|
|
|
41da01 |
@@ -1570,7 +1585,11 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
/* add some space to the end of the buffer as we will be adding a timespec to it */
|
|
|
41da01 |
if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
|
|
|
41da01 |
/* The client requests a File */
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
cl->fileTransfer.fd=open(filename1, O_RDONLY, 0744);
|
|
|
41da01 |
|
|
|
41da01 |
/*
|
|
|
41da01 |
@@ -1685,7 +1704,11 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
}
|
|
|
41da01 |
sizeHtmp = Swap32IfLE(sizeHtmp);
|
|
|
41da01 |
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
|
|
|
41da01 |
/* If the file exists... We can send a rfbFileChecksums back to the client before we send an rfbFileAcceptHeader */
|
|
|
41da01 |
/* TODO: Delta Transfer */
|
|
|
41da01 |
@@ -1814,7 +1837,12 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
|
|
|
41da01 |
switch (contentParam) {
|
|
|
41da01 |
case rfbCDirCreate: /* Client requests the creation of a directory */
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
+
|
|
|
41da01 |
retval = mkdir(filename1, 0755);
|
|
|
41da01 |
if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCDirCreate(\"%s\"->\"%s\") %s\n", buffer, filename1, (retval==-1?"Failed":"Success"));
|
|
|
41da01 |
/*
|
|
|
41da01 |
@@ -1823,7 +1851,12 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
return retval;
|
|
|
41da01 |
case rfbCFileDelete: /* Client requests the deletion of a file */
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
+
|
|
|
41da01 |
if (stat(filename1,&statbuf)==0)
|
|
|
41da01 |
{
|
|
|
41da01 |
if (S_ISDIR(statbuf.st_mode))
|
|
|
41da01 |
@@ -1841,8 +1874,18 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
{
|
|
|
41da01 |
/* Split into 2 filenames ('*' is a seperator) */
|
|
|
41da01 |
*p = '\0';
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
|
|
|
41da01 |
- rfbFilenameTranslate2UNIX(cl, p+1, filename2);
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
+
|
|
|
41da01 |
+ if (!rfbFilenameTranslate2UNIX(cl, p+1, filename2, sizeof(filename2)))
|
|
|
41da01 |
+ {
|
|
|
41da01 |
+ if (buffer!=NULL) free(buffer);
|
|
|
41da01 |
+ return FALSE;
|
|
|
41da01 |
+ }
|
|
|
41da01 |
+
|
|
|
41da01 |
retval = rename(filename1,filename2);
|
|
|
41da01 |
if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCFileRename(\"%s\"->\"%s\" -->> \"%s\"->\"%s\") %s\n", buffer, filename1, p+1, filename2, (retval==-1?"Failed":"Success"));
|
|
|
41da01 |
/*
|
|
|
41da01 |
|
|
|
41da01 |
commit f528072216dec01cee7ca35d94e171a3b909e677
|
|
|
41da01 |
Author: Nicolas Ruff <nruff@google.com>
|
|
|
41da01 |
Date: Mon Sep 1 14:51:07 2014 +0200
|
|
|
41da01 |
|
|
|
41da01 |
Fix stack-based buffer overflow in rfbFileTransferOffer message, FileTime processing
|
|
|
41da01 |
|
|
|
41da01 |
diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c
|
|
|
41da01 |
index 445331a..23532b0 100644
|
|
|
41da01 |
--- a/libvncserver/rfbserver.c
|
|
|
41da01 |
+++ b/libvncserver/rfbserver.c
|
|
|
41da01 |
@@ -1683,16 +1683,17 @@ rfbBool rfbProcessFileTransfer(rfbClientPtr cl, uint8_t contentType, uint8_t con
|
|
|
41da01 |
*/
|
|
|
41da01 |
if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
|
|
|
41da01 |
|
|
|
41da01 |
- /* Parse the FileTime */
|
|
|
41da01 |
+ /* Parse the FileTime
|
|
|
41da01 |
+ * TODO: FileTime is actually never used afterwards
|
|
|
41da01 |
+ */
|
|
|
41da01 |
p = strrchr(buffer, ',');
|
|
|
41da01 |
if (p!=NULL) {
|
|
|
41da01 |
*p = '\0';
|
|
|
41da01 |
- strcpy(szFileTime, p+1);
|
|
|
41da01 |
+ strncpy(szFileTime, p+1, sizeof(szFileTime));
|
|
|
41da01 |
+ szFileTime[sizeof(szFileTime)-1] = '\x00'; /* ensure NULL terminating byte is present, even if copy overflowed */
|
|
|
41da01 |
} else
|
|
|
41da01 |
szFileTime[0]=0;
|
|
|
41da01 |
|
|
|
41da01 |
-
|
|
|
41da01 |
-
|
|
|
41da01 |
/* Need to read in sizeHtmp */
|
|
|
41da01 |
if ((n = rfbReadExact(cl, (char *)&sizeHtmp, 4)) <= 0) {
|
|
|
41da01 |
if (n != 0)
|