From: Katy Feng Date: Fri, 23 Dec 2022 00:25:50 +0000 (-0800) Subject: Linux guest identification: Make the code more robust. X-Git-Tag: stable-12.2.0~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f884c52df4d03246e40715d771a23a9af17436e;p=thirdparty%2Fopen-vm-tools.git Linux guest identification: Make the code more robust. The code to read and parse the os-release data isn't doing a good job of protecting the tools daemon. Fixed this. - do not depend on sscanf. - bound the size of parameters; - better checking for syntax errors. --- diff --git a/open-vm-tools/lib/misc/hostinfoPosix.c b/open-vm-tools/lib/misc/hostinfoPosix.c index 3f105a4fc..7e33c670d 100644 --- a/open-vm-tools/lib/misc/hostinfoPosix.c +++ b/open-vm-tools/lib/misc/hostinfoPosix.c @@ -136,7 +136,6 @@ #endif #define LGPFX "HOSTINFO:" -#define MAX_LINE_LEN 128 #define SYSTEM_BITNESS_32 "i386" #define SYSTEM_BITNESS_64_SUN "amd64" @@ -160,27 +159,22 @@ static Atomic_Ptr hostinfoOSVersion; #define DISTRO_BUF_SIZE 1024 #if !defined(__APPLE__) && !defined(VMX86_SERVER) && !defined(USERWORLD) -typedef struct { - const char *name; - const char *scanString; -} DistroNameScan; - -static const DistroNameScan lsbFields[] = { - { "DISTRIB_ID=", "DISTRIB_ID=%s" }, - { "DISTRIB_RELEASE=", "DISTRIB_RELEASE=%s" }, - { "DISTRIB_CODENAME=", "DISTRIB_CODENAME=%s" }, - { "DISTRIB_DESCRIPTION=", "DISTRIB_DESCRIPTION=%s" }, - { NULL, NULL }, +static const char *lsbFields[] = { + "DISTRIB_ID=", + "DISTRIB_RELEASE=", + "DISTRIB_CODENAME=", + "DISTRIB_DESCRIPTION=", + NULL // MUST BE LAST }; -static const DistroNameScan osReleaseFields[] = { - { "PRETTY_NAME=", "PRETTY_NAME=%s" }, - { "NAME=", "NAME=%s" }, - { "VERSION_ID=", "VERSION_ID=%s" }, - { "BUILD_ID=", "BUILD_ID=%s" }, - { "VERSION=", "VERSION=%s" }, - { "CPE_NAME=", "CPE_NAME=%s" }, // NIST CPE specification - { NULL, NULL }, +static const char *osReleaseFields[] = { + "PRETTY_NAME=", + "NAME=", + "VERSION_ID=", + "BUILD_ID=", + "VERSION=", + "CPE_NAME=", // NIST CPE specification + NULL // MUST BE LAST }; typedef struct { @@ -1406,29 +1400,32 @@ HostinfoGetOSShortName(const char *distro, // IN: full distro name */ static char ** -HostinfoReadDistroFile(Bool osReleaseRules, // IN: osRelease rules - const char *filename, // IN: distro file - const DistroNameScan *values) // IN: search strings +HostinfoReadDistroFile(Bool osReleaseRules, // IN: osRelease rules + const char *fileName, // IN: distro file + const char *values[]) // IN: search strings { int i; + int fd; DynBuf b; int bufSize; struct stat st; - int fd = -1; + char lineBuf[DISTRO_BUF_SIZE]; + FILE *s = NULL; uint32 nArgs = 0; + Bool first = TRUE; Bool success = FALSE; char **result = NULL; char *distroOrig = NULL; /* It's OK for the file to not exist, don't warn for this. */ - if ((fd = Posix_Open(filename, O_RDONLY)) == -1) { + if ((fd = Posix_Open(fileName, O_RDONLY)) == -1) { return FALSE; } DynBuf_Init(&b); if (fstat(fd, &st)) { - Warning("%s: could not stat file '%s': %d\n", __FUNCTION__, filename, + Warning("%s: could not stat file '%s': %d\n", __FUNCTION__, fileName, errno); goto out; } @@ -1443,55 +1440,91 @@ HostinfoReadDistroFile(Bool osReleaseRules, // IN: osRelease rules distroOrig = Util_SafeCalloc(bufSize + 1, sizeof *distroOrig); if (read(fd, distroOrig, bufSize) != bufSize) { - Warning("%s: could not read file '%s': %d\n", __FUNCTION__, filename, + Warning("%s: could not read file '%s': %d\n", __FUNCTION__, fileName, errno); goto out; } distroOrig[bufSize] = '\0'; + lseek(fd, 0, SEEK_SET); + + s = fdopen(fd, "r"); + + if (s == NULL) { + Warning("%s: fdopen conversion failed.\n", __FUNCTION__); + goto out; + } + /* * Attempt to parse a file with one name=value pair per line. Values are * expected to embedded in double quotes. */ nArgs = 0; - for (i = 0; values[i].name != NULL; i++) { + for (i = 0; values[i] != NULL; i++) { nArgs++; } nArgs++; // For the appended version of the data result = Util_SafeCalloc(nArgs, sizeof(char *)); - for (i = 0; values[i].name != NULL; i++) { - const char *tmpDistroPos = strstr(distroOrig, values[i].name); + while (fgets(lineBuf, sizeof lineBuf, s) != NULL) { + for (i = 0; values[i] != NULL; i++) { + size_t len = strlen(values[i]); - if (tmpDistroPos != NULL) { - char distroPart[DISTRO_BUF_SIZE]; + if (strncmp(lineBuf, values[i], len) == 0) { + char *p; + char *data; - if (i != 0) { - DynBuf_Strcat(&b, " "); - } + if (lineBuf[len] == '"') { + data = &lineBuf[len + 1]; + p = strrchr(data, '"'); - sscanf(tmpDistroPos, values[i].scanString, distroPart); - if (distroPart[0] == '"') { - char *tmpMakeNull; - - tmpDistroPos += strlen(values[i].name) + 1; - tmpMakeNull = strchr(tmpDistroPos + 1 , '"'); - if (tmpMakeNull != NULL) { - *tmpMakeNull = '\0'; - DynBuf_Strcat(&b, tmpDistroPos); - result[i] = Util_SafeStrdup(tmpDistroPos); - *tmpMakeNull = '"' ; - } - } else { - DynBuf_Strcat(&b, distroPart); - result[i] = Util_SafeStrdup(distroPart); - } + if (p == NULL) { + Warning("%s: Invalid os-release file.", __FUNCTION__); + goto out; + } + } else { + data = &lineBuf[len]; + + p = strchr(data, '\n'); + + if (p == NULL) { + Warning("%s: os-release file line too long.", + __FUNCTION__); + goto out; + } + } + + *p = '\0'; + + if (p - data > MAX_DETAILED_FIELD_LEN) { + Warning("%s: Unexpectedly long data encountered; truncated.", + __FUNCTION__); + + p[MAX_DETAILED_FIELD_LEN - 1] = '\0'; + } + + if (!first) { + DynBuf_Strcat(&b, " "); + } + + DynBuf_Strcat(&b, data); + result[i] = Util_SafeStrdup(data); + + first = FALSE; + } } } + if (ferror(s)) { + Warning("%s: Error occurred while reading '%s'\n", __FUNCTION__, + fileName); + + goto out; + } + if (DynBuf_GetSize(&b) == 0) { /* * The distro identification file was not standards compliant. @@ -1524,7 +1557,9 @@ HostinfoReadDistroFile(Bool osReleaseRules, // IN: osRelease rules } out: - if (fd != -1) { + if (s != NULL) { + fclose(s); + } else if (fd != -1) { close(fd); } @@ -1663,12 +1698,11 @@ HostinfoOsRelease(char ***args) // OUT: { int score; - *args = HostinfoReadDistroFile(TRUE, "/etc/os-release", - &osReleaseFields[0]); + *args = HostinfoReadDistroFile(TRUE, "/etc/os-release", osReleaseFields); if (*args == NULL) { *args = HostinfoReadDistroFile(TRUE, "/usr/lib/os-release", - &osReleaseFields[0]); + osReleaseFields); } if (*args == NULL) { @@ -1771,7 +1805,7 @@ HostinfoLsb(char ***args) // OUT: for (i = 0; distroArray[i].filename != NULL; i++) { *args = HostinfoReadDistroFile(FALSE, distroArray[i].filename, - &lsbFields[0]); + lsbFields); if (*args != NULL) { break;