From: Kruti Date: Tue, 21 May 2024 05:58:13 +0000 (-0700) Subject: [Coverity]: Fixes for issues found from static application security testing X-Git-Tag: stable-12.5.0~59 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8cd75607fbb7b5923f791c3cd8270d88b73fbef2;p=thirdparty%2Fopen-vm-tools.git [Coverity]: Fixes for issues found from static application security testing Adding coverity escapes for false-positive issues. hgfsServerParameters.c -- 1 issue reported. issue: Overrunning array of 5 bytes at byte offset 5 by dereferencing pointer "newName". impact: False-Positive fix: suppress 'overrun-local' vmhgfs-fuse/file.c -- 2 issues reported. issue: Overrunning array of n bytes at byte offset n by dereferencing pointer "newNameP" (n is 17 and 5 respectively for those 2 locations where the issue occured). impact: False-Positive fix: suppress 'overrun-local' vmhgfs-fuse/link.c -- 2 issues reported. issue: Overrunning array of n bytes at byte offset n by dereferencing pointer "fileNameP" (n is 17 and 5 respectively for those 2 locations where the issue occured). impact: False-Positive fix: suppress 'overrun-local' vmhgfs-fuse/transport.c -- 1 issue reported. issue: uninit_use_in_call: Using uninitialized value "reply" while calling HgfsCompleteReq() function. impact: Bug fix: Remove function, it is unused/dead code (transport.h too). --- diff --git a/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c b/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c index 8b601829c..9836a3fea 100644 --- a/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c +++ b/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (C) 2010-2019 VMware, Inc. All rights reserved. + * Copyright (c) 2010-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -1968,6 +1969,13 @@ HgfsUnpackRenamePayloadV2(const HgfsRequestRenameV2 *requestV2, // IN: request p } else { newName = (const HgfsFileName *)((char *)(&requestV2->oldName + 1) + *cpOldNameLen); + /* + * The HgfsRequestRenameV2 structure overlay on the data has the old and + * new data interlaced rather. The newName pointer in the data is + * calculated as an offset from the oldName field. This confuses Coverity, + * there is no overrun here. + */ + /* coverity[overrun-local] */ if (!HgfsUnpackFileName(newName, extra, cpNewName, diff --git a/open-vm-tools/vmhgfs-fuse/file.c b/open-vm-tools/vmhgfs-fuse/file.c index f0323095a..b9b9a933f 100644 --- a/open-vm-tools/vmhgfs-fuse/file.c +++ b/open-vm-tools/vmhgfs-fuse/file.c @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (c) 2013,2018-2019, 2023 VMware, Inc. All rights reserved. + * Copyright (c) 2013-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -1038,6 +1039,11 @@ retry: result = -EINVAL; goto out; } + /* + * The usage of the space allocated in req early in the function is kept + * in reqSize. If oldName length was 0 we're not causing an overrun. + */ + /* coverity[overrun-local] */ newNameP->length = result; reqSize += result; newNameP->flags = 0; @@ -1060,6 +1066,11 @@ retry: result = -EINVAL; goto out; } + /* + * The usage of the space allocated in req early in the function is kept + * in reqSize. If oldName length was 0 we're not causing an overrun. + */ + /* coverity[overrun-local] */ newNameP->length = result; reqSize += result; } diff --git a/open-vm-tools/vmhgfs-fuse/link.c b/open-vm-tools/vmhgfs-fuse/link.c index 3a6fc912c..9a607ca51 100644 --- a/open-vm-tools/vmhgfs-fuse/link.c +++ b/open-vm-tools/vmhgfs-fuse/link.c @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (C) 2013,2019 VMware, Inc. All rights reserved. + * Copyright (c) 2013-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -90,6 +91,12 @@ HgfsPackSymlinkCreateRequest(const char* symlink, // IN: path of the link LOG(6, ("Target name: \"%s\"\n", fileNameP->name)); /* Convert target name to CPName-lite format. */ CPNameLite_ConvertTo(fileNameP->name, targetNameBytes - 1, '/'); + /* + * The req size is always sufficient to hold the request data. + * There is no overrun here, coverity has issue with how the data is + * packed (name fields data are interlaced). + */ + /* coverity[overrun-local] */ fileNameP->length = targetNameBytes - 1; fileNameP->flags = 0; fileNameP->fid = HGFS_INVALID_HANDLE; @@ -125,6 +132,12 @@ HgfsPackSymlinkCreateRequest(const char* symlink, // IN: path of the link LOG(6, ("Target name: \"%s\"\n", fileNameP->name)); /* Convert target name to CPName-lite format. */ CPNameLite_ConvertTo(fileNameP->name, targetNameBytes - 1, '/'); + /* + * The req size is always sufficient to hold the request data. + * There is no overrun here, coverity has issue with how the data is + * packed (name fields data are interlaced). + */ + /* coverity[overrun-local] */ fileNameP->length = targetNameBytes - 1; break; } diff --git a/open-vm-tools/vmhgfs-fuse/transport.c b/open-vm-tools/vmhgfs-fuse/transport.c index 22fa5a070..4ba451a5d 100644 --- a/open-vm-tools/vmhgfs-fuse/transport.c +++ b/open-vm-tools/vmhgfs-fuse/transport.c @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (C) 2013,2019 VMware, Inc. All rights reserved. + * Copyright (c) 2013-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -266,42 +267,6 @@ HgfsTransportProcessPacket(char *receivedPacket, //IN: received packet } -/* - *---------------------------------------------------------------------- - * - * HgfsTransportBeforeExitingRecvThread -- - * - * The cleanup work to do before the recv thread exits, including - * completing pending requests with error. - * - * Results: - * None - * - * Side effects: - * None - * - *---------------------------------------------------------------------- - */ - -void -HgfsTransportBeforeExitingRecvThread(void) -{ - struct list_head *cur, *next; - - /* Walk through gHgfsPendingRequests queue and reply them with error. */ - pthread_mutex_lock(&gHgfsPendingRequestsLock); - list_for_each_safe(cur, next, &gHgfsPendingRequests) { - HgfsReq *req; - HgfsReply reply; - - req = list_entry(cur, HgfsReq, list); - LOG(6, ("Injecting error reply to req id: %d\n", req->id)); - HgfsCompleteReq(req, (char *)&reply, sizeof reply); - } - pthread_mutex_unlock(&gHgfsPendingRequestsLock); -} - - /* *---------------------------------------------------------------------- * diff --git a/open-vm-tools/vmhgfs-fuse/transport.h b/open-vm-tools/vmhgfs-fuse/transport.h index e8d92e05b..49b8677c5 100644 --- a/open-vm-tools/vmhgfs-fuse/transport.h +++ b/open-vm-tools/vmhgfs-fuse/transport.h @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (C) 2013 VMware, Inc. All rights reserved. + * Copyright (c) 2013-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -58,6 +59,5 @@ void HgfsTransportExit(void); int HgfsTransportSendRequest(HgfsReq *req); void HgfsTransportProcessPacket(char *receivedPacket, size_t receivedSize); -void HgfsTransportBeforeExitingRecvThread(void); #endif // _HGFS_DRIVER_TRANSPORT_H_