From: Oliver Kurth Date: Fri, 15 Sep 2017 18:23:02 +0000 (-0700) Subject: Hgfs Server: write op move argument checks security fixes X-Git-Tag: stable-10.2.0~549 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=245bcf456f0376344eb71c0edbbfbba4c2e7e3b3;p=thirdparty%2Fopen-vm-tools.git Hgfs Server: write op move argument checks security fixes Consolidate the argument verifications to the common code as these are often duplicated. This will now explicitly perform the Hgfs write operation argument verification in the common handler before calling the platform specific code to actually perform the write. --- diff --git a/open-vm-tools/lib/hgfsServer/hgfsServer.c b/open-vm-tools/lib/hgfsServer/hgfsServer.c index 07ee67179..f5187f4de 100644 --- a/open-vm-tools/lib/hgfsServer/hgfsServer.c +++ b/open-vm-tools/lib/hgfsServer/hgfsServer.c @@ -6371,9 +6371,36 @@ HgfsServerValidateWrite(HgfsInputParam *input, // IN: Input params /* * TBD - * - validate the packet size with the header, write request and write data - * - map the file handle, and extract the details of the write e.g. writing + */ + + /* + * Now map the file handle, and extract the details of the write e.g. writing * sequentially or appending + * + * Validate the file handle by retrieving it possibly from the cache. */ + status = HgfsPlatformGetFd(writeHandle, input->session, + ((flags & HGFS_WRITE_APPEND) ? TRUE : FALSE), + &writeFileDesc); + if (status != HGFS_ERROR_SUCCESS) { + LOG(4, ("%s: Error: arg validation handle -> %d.\n", + __FUNCTION__, status)); + goto exit; + } + + if (!HgfsHandleIsSequentialOpen(writeHandle, input->session, &sequentialHandle)) { + status = HGFS_ERROR_INVALID_HANDLE; + LOG(4, ("%s: Could not get sequential open status\n", __FUNCTION__)); + goto exit; + } + +#if defined(__APPLE__) + if (!HgfsHandle2AppendFlag(writeHandle, input->session, &appendHandle)) { + status = HGFS_ERROR_INVALID_HANDLE; + LOG(4, ("%s: Could not get append mode\n", __FUNCTION__)); + goto exit; + } +#endif exit: *writefd = writeFileDesc; @@ -6427,9 +6454,8 @@ HgfsServerWrite(HgfsInputParam *input) // IN: Input params } /* - * Validate the read arguments with the data and reply buffers to ensure - * there isn't a malformed request or we read more data than the buffer can - * hold. + * Validate the write arguments with the data and request buffers to ensure + * there isn't a malformed request or we try to write more data than is in the buffer. */ status = HgfsServerValidateWrite(input, writeFile, @@ -6457,11 +6483,13 @@ HgfsServerWrite(HgfsInputParam *input) // IN: Input params } } - status = HgfsPlatformWriteFile(writeFile, + status = HgfsPlatformWriteFile(writeFd, input->session, writeOffset, writeSize, writeFlags, + writeSequential, + writeAppend, writeData, &writtenSize); if (HGFS_ERROR_SUCCESS != status) { diff --git a/open-vm-tools/lib/hgfsServer/hgfsServerInt.h b/open-vm-tools/lib/hgfsServer/hgfsServerInt.h index 7dd709ffd..f9d046ca4 100644 --- a/open-vm-tools/lib/hgfsServer/hgfsServerInt.h +++ b/open-vm-tools/lib/hgfsServer/hgfsServerInt.h @@ -748,11 +748,13 @@ HgfsPlatformReadFile(fileDesc readFile, // IN: file descriptor void* payload, // OUT: buffer for the read data uint32 *actualSize); // OUT: actual length read HgfsInternalStatus -HgfsPlatformWriteFile(HgfsHandle file, // IN: Hgfs file handle +HgfsPlatformWriteFile(fileDesc writeFile, // IN: file descriptor HgfsSessionInfo *session, // IN: session info uint64 writeOffset, // IN: file offset to write to uint32 writeDataSize, // IN: length of data to write HgfsWriteFlags writeFlags, // IN: write flags + Bool writeSequential, // IN: write is sequential + Bool writeAppend, // IN: write is appended const void *writeData, // IN: data to be written uint32 *writtenSize); // OUT: byte length written HgfsInternalStatus diff --git a/open-vm-tools/lib/hgfsServer/hgfsServerLinux.c b/open-vm-tools/lib/hgfsServer/hgfsServerLinux.c index 9c6ec1e75..3ab20adfa 100644 --- a/open-vm-tools/lib/hgfsServer/hgfsServerLinux.c +++ b/open-vm-tools/lib/hgfsServer/hgfsServerLinux.c @@ -4240,36 +4240,21 @@ HgfsPlatformReadFile(fileDesc file, // IN: file descriptor */ HgfsInternalStatus -HgfsPlatformWriteFile(HgfsHandle file, // IN: Hgfs file handle +HgfsPlatformWriteFile(fileDesc writeFd, // IN: file descriptor HgfsSessionInfo *session, // IN: session info uint64 writeOffset, // IN: file offset to write to uint32 writeDataSize, // IN: length of data to write HgfsWriteFlags writeFlags, // IN: write flags + Bool writeSequential, // IN: write is sequential + Bool writeAppend, // IN: write is appended const void *writeData, // IN: data to be written uint32 *writtenSize) // OUT: actual length written { - HgfsInternalStatus status; - int writeFd; + HgfsInternalStatus status = 0; int error = 0; - Bool writeSequential; LOG(4, ("%s: write fh %u offset %"FMT64"u, count %u\n", - __FUNCTION__, file, writeOffset, writeDataSize)); - - /* Get the file desriptor from the cache */ - status = HgfsPlatformGetFd(file, session, - ((writeFlags & HGFS_WRITE_APPEND) ? TRUE : FALSE), - &writeFd); - - if (status != 0) { - LOG(4, ("%s: Could not get file descriptor\n", __FUNCTION__)); - return status; - } - - if (!HgfsHandleIsSequentialOpen(file, session, &writeSequential)) { - LOG(4, ("%s: Could not get sequential open status\n", __FUNCTION__)); - return EBADF; - } + __FUNCTION__, writeFd, writeOffset, writeDataSize)); #if !defined(sun) if (!writeSequential) { @@ -4289,13 +4274,6 @@ HgfsPlatformWriteFile(HgfsHandle file, // IN: Hgfs file handle } #elif defined(__APPLE__) { - Bool writeAppend; - - if (!HgfsHandle2AppendFlag(file, session, &writeAppend)) { - LOG(4, ("%s: Could not get append mode\n", __FUNCTION__)); - return EBADF; - } - /* Write to the file. */ if (writeSequential || writeAppend) { error = write(writeFd, writeData, writeDataSize);