Merge pull request #2053 from jimklimov/issue-2052

Fix segfault in NUT clients with empty host/device name components

Author: jimklimov

NEWS.adoc

@@ -114,6 +114,10 @@ as part of https://github.com/networkupstools/nut/issues/1410 solution.
      seems to have confused the MGE SHUT (Serial HID UPS Transfer) driver
      support [#2022]
 
+ - An issue was identified which could cause `libupsclient` parser of device
+   and host names to crash upon bad inputs (e.g. poorly resolved environment
+   variables in scripts). Now it should fail more gracefully [#2052]
+
  - New `configure --enable-inplace-runtime` option should set default values
    for `--sysconfdir`, `--with-user` and `--with-group` options to match an
    existing NUT deployment -- for users who are trying if a custom build

clients/upsclient.c

@@ -1161,7 +1161,7 @@ int upscli_tryconnect(UPSCONN_t *ups, const char *host, uint16_t port, int flags
 
 	pconf_init(&ups->pc_ctx, NULL);
 
-	ups->host = strdup(host);
+	ups->host = xstrdup(host);
 
 	if (!ups->host) {
 		ups->upserror = UPSCLI_ERR_NOMEM;
@@ -1618,22 +1618,45 @@ int upscli_splitname(const char *buf, char **upsname, char **hostname, uint16_t
 
 	s = strchr(tmp, '@');
 
-	if ((*upsname = strdup(strtok_r(tmp, "@", &last))) == NULL) {
-		fprintf(stderr, "upscli_splitname: strdup failed\n");
+	/* someone passed a "@hostname" string? */
+	if (s == tmp) {
+		fprintf(stderr, "upscli_splitname: got empty upsname string\n");
 		return -1;
 	}
 
+	if ((*upsname = xstrdup(strtok_r(tmp, "@", &last))) == NULL) {
+		fprintf(stderr, "upscli_splitname: xstrdup failed\n");
+		return -1;
+	}
+
+	/* someone passed a "@hostname" string (take two)? */
+	if (!**upsname) {
+		fprintf(stderr, "upscli_splitname: got empty upsname string\n");
+		return -1;
+	}
+
+	/*
+	fprintf(stderr, "upscli_splitname3: got buf='%s', tmp='%s', upsname='%s', possible hostname:port='%s'\n",
+		NUT_STRARG(buf), NUT_STRARG(tmp), NUT_STRARG(*upsname), NUT_STRARG((s ? s+1 : s)));
+	*/
+
 	/* only a upsname is specified, fill in defaults */
 	if (s == NULL) {
-		if ((*hostname = strdup("localhost")) == NULL) {
-			fprintf(stderr, "upscli_splitname: strdup failed\n");
+		if ((*hostname = xstrdup("localhost")) == NULL) {
+			fprintf(stderr, "upscli_splitname: xstrdup failed\n");
 			return -1;
 		}
 
 		*port = PORT;
 		return 0;
 	}
 
+	/* someone passed a "upsname@" string? */
+	if (!(*(s+1))) {
+		fprintf(stderr, "upscli_splitname: got the @ separator and then an empty hostname[:port] string\n");
+		return -1;
+	}
+
 	return upscli_splitaddr(s+1, hostname, port);
 }
 
@@ -1659,8 +1682,8 @@ int upscli_splitaddr(const char *buf, char **hostname, uint16_t *port)
 			return -1;
 		}
 
-		if ((*hostname = strdup(strtok_r(tmp+1, "]", &last))) == NULL) {
-			fprintf(stderr, "upscli_splitaddr: strdup failed\n");
+		if ((*hostname = xstrdup(strtok_r(tmp+1, "]", &last))) == NULL) {
+			fprintf(stderr, "upscli_splitaddr: xstrdup failed\n");
 			return -1;
 		}
 
@@ -1672,8 +1695,8 @@ int upscli_splitaddr(const char *buf, char **hostname, uint16_t *port)
 	} else {
 		s = strchr(tmp, ':');
 
-		if ((*hostname = strdup(strtok_r(tmp, ":", &last))) == NULL) {
-			fprintf(stderr, "upscli_splitaddr: strdup failed\n");
+		if ((*hostname = xstrdup(strtok_r(tmp, ":", &last))) == NULL) {
+			fprintf(stderr, "upscli_splitaddr: xstrdup failed\n");
 			return -1;
 		}
 

common/common.c

@@ -1738,7 +1738,14 @@ void *xrealloc(void *ptr, size_t size)
 
 char *xstrdup(const char *string)
 {
-	char *p = strdup(string);
+	char *p;
+
+	if (string == NULL) {
+		upsdebugx(1, "%s: got null input", __func__);
+		return NULL;
+	}
+
+	p = strdup(string);
 
 	if (p == NULL)
 		fatal_with_errno(EXIT_FAILURE, "%s", oom_msg);