From 5987a7b1af09e97271be3da74d336a64435e759a Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Thu, 23 Nov 2017 21:25:26 +0100 Subject: Dialog.c,Display.c,Font.c,NXdixfonts.c: don't use hardcoded string buffer lengths --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index 4930baee2..5943e538e 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1150,9 +1150,9 @@ void nxagentOpenDisplay(int argc, char *argv[]) if (*nxagentDisplayName == '\0') { - strncpy(nxagentDisplayName, XDisplayName(NULL), 1023); + strncpy(nxagentDisplayName, XDisplayName(NULL), sizeof(nxagentDisplayName) - 1); - nxagentDisplayName[1023] = '\0'; + nxagentDisplayName[sizeof(nxagentDisplayName) - 1] = '\0'; } nxagentCloseDisplay(); @@ -1862,7 +1862,7 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, singlePath[strlen(singlePath)- 1] = 0; } - if (strlen(singlePath) + strlen(iconName) + 1 < PATH_MAX) + if (strlen(singlePath) + strlen(iconName) + 1 < sizeof(singlePath)<) { strncat(singlePath, slash, 1); strcat(singlePath, iconName); -- cgit v1.2.3 From 4a345786c6ee3b00882f015a7ac7d1d3215c0b9f Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Fri, 24 Nov 2017 00:25:25 +0100 Subject: Dialog.c,Display.c,NXdixfonts.c: replace strncpy() by snprintf where appropriate --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index 5943e538e..f523dacde 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1150,9 +1150,7 @@ void nxagentOpenDisplay(int argc, char *argv[]) if (*nxagentDisplayName == '\0') { - strncpy(nxagentDisplayName, XDisplayName(NULL), sizeof(nxagentDisplayName) - 1); - - nxagentDisplayName[sizeof(nxagentDisplayName) - 1] = '\0'; + snprintf(nxagentDisplayName, sizeof(nxagentDisplayName), "%s", XDisplayName(NULL)); } nxagentCloseDisplay(); @@ -1846,7 +1844,7 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, { strncpy(singlePath, path, (unsigned long)(end - path)); - singlePath[(unsigned long)(end - path)] = 0; + singlePath[(unsigned long)(end - path)] = '\0'; path = end + 1; } @@ -1859,7 +1857,7 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, if (singlePath[strlen(singlePath)- 1] == slash[0]) { - singlePath[strlen(singlePath)- 1] = 0; + singlePath[strlen(singlePath)- 1] = '\0'; } if (strlen(singlePath) + strlen(iconName) + 1 < sizeof(singlePath)<) @@ -1907,8 +1905,8 @@ Bool nxagentMakeIcon(Display *display, Pixmap *nxIcon, Pixmap *nxMask) agentIconData=nxagentIconData; } - - snprintf(default_path, PATH_MAX-1, "/usr/NX/share/images/%s", agent_icon_name); + /* FIXME: use a compile time define here, /usr/NX is a nomachine path */ + snprintf(default_path, sizeof(default_path), "/usr/NX/share/images/%s", agent_icon_name); if ((icon_fp = fopen(default_path, "r")) == NULL) { -- cgit v1.2.3 From 3de6bc7490ff6907cd0203c6143a75588458dbb9 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Thu, 23 Nov 2017 23:18:44 +0100 Subject: Dialog.c: fix possible buffer overflows Fix write past the end of singlePath if PATH contains dirs longer than PATH_MAX. --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index f523dacde..d4e032046 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1842,6 +1842,13 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, if (end != NULL) { + if ((end - path) > sizeof(singlePath) - 1) + { + fprintf(stderr, "Warning: Path too long - ignored.\n"); + path = end + 1; + continue; + } + strncpy(singlePath, path, (unsigned long)(end - path)); singlePath[(unsigned long)(end - path)] = '\0'; @@ -1850,6 +1857,12 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, } else { + if (strlen(path) > sizeof(singlePath) - 1) + { + fprintf(stderr, "Error: Path too long.\n"); + return NULL; + } + strcpy(singlePath, path); breakLoop = 1; -- cgit v1.2.3 From ea1e0bea3c41f5c8ad8fc3e22ecd8f2d44e82685 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Tue, 2 Jan 2018 19:23:20 +0100 Subject: Replace hardcoded string lengths by macros --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index d4e032046..c9c2f4e7a 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1150,7 +1150,7 @@ void nxagentOpenDisplay(int argc, char *argv[]) if (*nxagentDisplayName == '\0') { - snprintf(nxagentDisplayName, sizeof(nxagentDisplayName), "%s", XDisplayName(NULL)); + snprintf(nxagentDisplayName, NXAGENTDISPLAYNAMELENGTH, "%s", XDisplayName(NULL)); } nxagentCloseDisplay(); -- cgit v1.2.3 From 19a3918a7216ac1006bdfd96239fafc6eb97d523 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Tue, 2 Jan 2018 19:24:49 +0100 Subject: Display.c: pass down buffer size to nxagentLookForIconFile Also comment the code and convert error messages to warnings. --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 46 +++++++++++++--------------- 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index c9c2f4e7a..b78530e37 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1815,12 +1815,10 @@ FIXME: Is this needed? } static FILE *nxagentLookForIconFile(char *iconName, const char *permission, - char *return_path) + char *return_path, int return_path_size) { char *path; - char *end; char singlePath[PATH_MAX]; - int breakLoop; FILE *fptr = NULL; #ifdef WIN32 @@ -1836,56 +1834,56 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, return NULL; } - for(breakLoop = 0; breakLoop == 0 && fptr == NULL; ) + for (int breakLoop = False; breakLoop == False && fptr == NULL; ) { - end = strchr(path, separator); + char *end = strchr(path, separator); + /* separator found */ if (end != NULL) { if ((end - path) > sizeof(singlePath) - 1) { - fprintf(stderr, "Warning: Path too long - ignored.\n"); + fprintf(stderr, "Warning: PATH component too long - ignoring it.\n"); path = end + 1; continue; } - strncpy(singlePath, path, (unsigned long)(end - path)); - - singlePath[(unsigned long)(end - path)] = '\0'; - + snprintf(singlePath, (unsigned long)(end - path + 1), "%s", path); path = end + 1; } else { if (strlen(path) > sizeof(singlePath) - 1) { - fprintf(stderr, "Error: Path too long.\n"); + fprintf(stderr, "Warning: PATH component too long - ignoring it.\n"); return NULL; } - strcpy(singlePath, path); + snprintf(singlePath, sizeof(singlePath), "%s", path); - breakLoop = 1; + breakLoop = True; } - if (singlePath[strlen(singlePath)- 1] == slash[0]) + /* cut off trailing slashes, if any */ + while (singlePath[strlen(singlePath) - 1] == slash[0]) { - singlePath[strlen(singlePath)- 1] = '\0'; + singlePath[strlen(singlePath) - 1] = '\0'; } - if (strlen(singlePath) + strlen(iconName) + 1 < sizeof(singlePath)<) + /* append slash and icon name */ + if (strlen(singlePath) + strlen(iconName) + 1 < sizeof(singlePath)) { strncat(singlePath, slash, 1); strcat(singlePath, iconName); if ((fptr = fopen(singlePath, permission)) != NULL) { - strcpy(return_path, singlePath); + snprintf(return_path, return_path_size, "%s", singlePath); } } else { - fprintf(stderr, "Error: Path too long.\n"); + fprintf(stderr, "Warning: Icon path too long.\n"); } } @@ -1909,13 +1907,13 @@ Bool nxagentMakeIcon(Display *display, Pixmap *nxIcon, Pixmap *nxMask) */ if(nxagentX2go) { - agent_icon_name=X2GOAGENT_ICON_NAME; - agentIconData=x2goagentIconData; + agent_icon_name = X2GOAGENT_ICON_NAME; + agentIconData = x2goagentIconData; } else { - agent_icon_name=NXAGENT_ICON_NAME; - agentIconData=nxagentIconData; + agent_icon_name = NXAGENT_ICON_NAME; + agentIconData = nxagentIconData; } /* FIXME: use a compile time define here, /usr/NX is a nomachine path */ @@ -1923,7 +1921,7 @@ Bool nxagentMakeIcon(Display *display, Pixmap *nxIcon, Pixmap *nxMask) if ((icon_fp = fopen(default_path, "r")) == NULL) { - icon_fp = nxagentLookForIconFile(agent_icon_name, "r", icon_path); + icon_fp = nxagentLookForIconFile(agent_icon_name, "r", icon_path, sizeof(icon_path)); if (icon_fp != NULL) { @@ -1935,7 +1933,7 @@ Bool nxagentMakeIcon(Display *display, Pixmap *nxIcon, Pixmap *nxMask) { fclose (icon_fp); success = True; - strcpy(icon_path, default_path); + snprintf(icon_path, sizeof(icon_path), "%s", default_path); } if (success) -- cgit v1.2.3 From 525e151681ec93657f027645ff0d8e0d487d6abf Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 3 Jan 2018 00:06:39 +0100 Subject: Error.c: replace strcpy/strcat by snprintf --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index b78530e37..31efab8c3 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1873,8 +1873,7 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, /* append slash and icon name */ if (strlen(singlePath) + strlen(iconName) + 1 < sizeof(singlePath)) { - strncat(singlePath, slash, 1); - strcat(singlePath, iconName); + snprintf(singlePath + strlen(singlePath), sizeof(singlePath), "%s%s", slash, iconName); if ((fptr = fopen(singlePath, permission)) != NULL) { -- cgit v1.2.3 From 23c36c2d2c0d1ea85c1b638d71267c28522c08cd Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 3 Jan 2018 22:28:43 +0100 Subject: Display.c: drop helper variable in loop --- nx-X11/programs/Xserver/hw/nxagent/Display.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Display.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Display.c b/nx-X11/programs/Xserver/hw/nxagent/Display.c index 31efab8c3..4f2e3c6d8 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Display.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Display.c @@ -1834,9 +1834,9 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, return NULL; } - for (int breakLoop = False; breakLoop == False && fptr == NULL; ) + for (char *end = path; end != NULL && fptr == NULL; ) { - char *end = strchr(path, separator); + end = strchr(path, separator); /* separator found */ if (end != NULL) @@ -1860,8 +1860,6 @@ static FILE *nxagentLookForIconFile(char *iconName, const char *permission, } snprintf(singlePath, sizeof(singlePath), "%s", path); - - breakLoop = True; } /* cut off trailing slashes, if any */ -- cgit v1.2.3