From 6e98e35cf24016c9789be26d33d918f6e0e3c9a1 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Thu, 28 Dec 2017 11:19:44 +0100 Subject: nxcomp: drop strncpy in favour of snprintf with very few exceptions which require careful thinking ;-) --- nxcomp/src/Auth.cpp | 14 ++++---------- nxcomp/src/Children.cpp | 19 +++++++++---------- nxcomp/src/Loop.cpp | 44 ++++++++++++++++++++------------------------ 3 files changed, 33 insertions(+), 44 deletions(-) (limited to 'nxcomp/src') diff --git a/nxcomp/src/Auth.cpp b/nxcomp/src/Auth.cpp index d398f5f85..87955f6ab 100644 --- a/nxcomp/src/Auth.cpp +++ b/nxcomp/src/Auth.cpp @@ -212,16 +212,14 @@ int Auth::getCookie() if (environment != NULL && *environment != '\0') { - strncpy(file_, environment, DEFAULT_STRING_LIMIT - 1); + snprintf(file_, DEFAULT_STRING_LIMIT, "%s", environment); } else { - snprintf(file_, DEFAULT_STRING_LIMIT - 1, "%s/.Xauthority", + snprintf(file_, DEFAULT_STRING_LIMIT, "%s/.Xauthority", control -> HomePath); } - *(file_ + DEFAULT_STRING_LIMIT - 1) = '\0'; - #ifdef TEST *logofs << "Auth: Using X authorization file '" << file_ << "'.\n" << logofs_flush; @@ -242,18 +240,14 @@ int Auth::getCookie() #if defined(__CYGWIN32__) - snprintf(command, DEFAULT_STRING_LIMIT - 1, + snprintf(command, DEFAULT_STRING_LIMIT, "%s/bin/nxauth", control -> SystemPath); - *(command + DEFAULT_STRING_LIMIT - 1) = '\0'; - #elif defined(__APPLE__) - snprintf(command, DEFAULT_STRING_LIMIT - 1, + snprintf(command, DEFAULT_STRING_LIMIT, "%s/nxauth", control -> SystemPath); - *(command + DEFAULT_STRING_LIMIT - 1) = '\0'; - #else strcpy(command, "xauth"); diff --git a/nxcomp/src/Children.cpp b/nxcomp/src/Children.cpp index 9486f189a..e586292da 100644 --- a/nxcomp/src/Children.cpp +++ b/nxcomp/src/Children.cpp @@ -275,12 +275,14 @@ int NXTransDialog(const char *caption, const char *message, #ifdef __APPLE__ + // FIXME: missing length limitation! strcat(newPath, "/Applications/NX Client for OSX.app/Contents/MacOS:"); #endif #ifdef __CYGWIN32__ + // FIXME: missing length limitation! strcat(newPath, ".:"); #endif @@ -289,9 +291,8 @@ int NXTransDialog(const char *caption, const char *message, char *oldPath = getenv("PATH"); - strncpy(newPath + newLength, oldPath, DEFAULT_STRING_LIMIT - newLength - 1); - - newPath[DEFAULT_STRING_LIMIT - 1] = '\0'; + // FIXME: check if strncat would be better here + snprintf(newPath + newLength, DEFAULT_STRING_LIMIT - newLength, "%s", oldPath); #ifdef WARNING *logofs << "NXTransDialog: WARNING! Trying with path '" @@ -427,17 +428,13 @@ int NXTransClient(const char* display) #ifdef __sun - snprintf(newDisplay, DISPLAY_LENGTH_LIMIT - 1, "DISPLAY=%s", display); - - newDisplay[DISPLAY_LENGTH_LIMIT - 1] = '\0'; + snprintf(newDisplay, DISPLAY_LENGTH_LIMIT, "DISPLAY=%s", display); putenv(newDisplay); #else - strncpy(newDisplay, display, DISPLAY_LENGTH_LIMIT - 1); - - newDisplay[DISPLAY_LENGTH_LIMIT - 1] = '\0'; + snprintf(newDisplay, DISPLAY_LENGTH_LIMIT, "%s", display); setenv("DISPLAY", newDisplay, 1); @@ -467,6 +464,7 @@ int NXTransClient(const char* display) if (i == 0) { + // FIXME: code dpulication: this whole block is duplicated in NXTransDialog strcpy(command, "nxclient"); char newPath[DEFAULT_STRING_LIMIT]; @@ -489,7 +487,8 @@ int NXTransClient(const char* display) char *oldPath = getenv("PATH"); - strncpy(newPath + newLength, oldPath, DEFAULT_STRING_LIMIT - newLength - 1); + // FIXME: check if strncat would be better here + snprintf(newPath + newLength, DEFAULT_STRING_LIMIT - newLength, "%s", oldPath); newPath[DEFAULT_STRING_LIMIT - 1] = '\0'; diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp index b0b778a14..6b903789b 100644 --- a/nxcomp/src/Loop.cpp +++ b/nxcomp/src/Loop.cpp @@ -4044,7 +4044,7 @@ void SetupDisplaySocket(int &addr_family, sockaddr *&addr, if (useLaunchdSocket == 1) { - strncpy(unixSocketName, displayHost, DEFAULT_STRING_LENGTH - 1); + snprintf(unixSocketName, DEFAULT_STRING_LENGTH, "%s", displayHost); } #endif @@ -7923,11 +7923,11 @@ int ParseEnvironmentOptions(const char *env, int force) if (strcasecmp(name, "options") == 0) { - strncpy(fileOptions, value, DEFAULT_STRING_LENGTH - 1); + snprintf(fileOptions, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "display") == 0) { - strncpy(displayHost, value, DEFAULT_STRING_LENGTH - 1); + snprintf(displayHost, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "link") == 0) { @@ -7983,7 +7983,7 @@ int ParseEnvironmentOptions(const char *env, int force) } else { - strncpy(sessionType, value, DEFAULT_STRING_LENGTH - 1); + snprintf(sessionType, DEFAULT_STRING_LENGTH, "%s", value); } } } @@ -8036,7 +8036,7 @@ int ParseEnvironmentOptions(const char *env, int force) return -1; } - strncpy(acceptHost, value, DEFAULT_STRING_LENGTH - 1); + snprintf(acceptHost, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "connect") == 0) { @@ -8074,7 +8074,7 @@ int ParseEnvironmentOptions(const char *env, int force) } else if (strcasecmp(name, "session") == 0) { - strncpy(sessionFileName, value, DEFAULT_STRING_LENGTH - 1); + snprintf(sessionFileName, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "errors") == 0) { @@ -8085,27 +8085,27 @@ int ParseEnvironmentOptions(const char *env, int force) // the same name. // - strncpy(errorsFileName, value, DEFAULT_STRING_LENGTH - 1); + snprintf(errorsFileName, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "root") == 0) { - strncpy(rootDir, value, DEFAULT_STRING_LENGTH - 1); + snprintf(rootDir, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "id") == 0) { - strncpy(sessionId, value, DEFAULT_STRING_LENGTH - 1); + snprintf(sessionId, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "stats") == 0) { control -> EnableStatistics = 1; - strncpy(statsFileName, value, DEFAULT_STRING_LENGTH - 1); + snprintf(statsFileName, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "cookie") == 0) { LowercaseArg("local", name, value); - strncpy(authCookie, value, DEFAULT_STRING_LENGTH - 1); + snprintf(authCookie, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "nodelay") == 0) { @@ -8334,7 +8334,7 @@ int ParseEnvironmentOptions(const char *env, int force) } else if (strcasecmp(name, "font") == 0) { - strncpy(fontPort, value, DEFAULT_STRING_LENGTH - 1); + snprintf(fontPort, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "slave") == 0) { @@ -8439,7 +8439,7 @@ int ParseEnvironmentOptions(const char *env, int force) } else if (strcasecmp(name, "product") == 0) { - strncpy(productName, value, DEFAULT_STRING_LENGTH - 1); + snprintf(productName, DEFAULT_STRING_LENGTH, "%s", value); } else if (strcasecmp(name, "rootless") == 0 || strcasecmp(name, "geometry") == 0 || @@ -8529,7 +8529,7 @@ int ParseEnvironmentOptions(const char *env, int force) if (*optionsFileName == '\0') { - strncpy(optionsFileName, value, DEFAULT_STRING_LENGTH - 1); + snprintf(optionsFileName, DEFAULT_STRING_LENGTH, "%s", value); nxinfo << "Loop: Assuming name of options file '" << optionsFileName << "'.\n" @@ -9249,7 +9249,7 @@ int ParseRemoteOptions(char *opts) } else { - strncpy(sessionType, value, DEFAULT_STRING_LENGTH - 1); + snprintf(sessionType, DEFAULT_STRING_LENGTH, "%s", value); } } @@ -12719,6 +12719,7 @@ int ParseHostOption(const char *opt, char *host, long &port) char newHost[DEFAULT_STRING_LENGTH] = { 0 }; + // opt cannot be longer than DEFAULT_STRING_LENGTH, this is checked above strncpy(newHost, opt, strlen(opt) - strlen(separator)); *(newHost + strlen(opt) - strlen(separator)) = '\0'; @@ -13491,10 +13492,8 @@ int ParseArg(const char *type, const char *name, const char *value) char *string = new char[strlen(value)]; - strncpy(string, value, strlen(value) - 1); - - *(string + (strlen(value) - 1)) = '\0'; - + // copy value but cut off the last character + snprintf(string, strlen(value), "%s", value); nxinfo << "Loop: Parsing integer option '" << name << "' from string '" << string << "' with base set to "; @@ -13512,18 +13511,15 @@ int ParseArg(const char *type, const char *name, const char *value) nxinfo_append << ".\n" << std::flush; - double result = atof(string) * base; + delete [] string; + if (result < 0 || result > (((unsigned) -1) >> 1)) { - delete [] string; - return -1; } - delete [] string; - nxinfo << "Loop: Integer option parsed to '" << (int) result << "'.\n" << std::flush; -- cgit v1.2.3