fix errors detected by Coverity Scan

buffer.c:itostr() undefined behavior taking modulus of negative number

additional minor code changes made to quiet other coverity warnings
(false positives)

10 files changed, 47 insertions, 46 deletions

diff --git a/src/buffer.c b/src/buffer.c
index 97514357..b79ea992 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -270,23 +270,13 @@ static char* utostr(char * const buf_end, uintmax_t val) {
 }
 
 static char* itostr(char * const buf_end, intmax_t val) {
-	char *cur = buf_end;
-	if (val >= 0) return utostr(buf_end, (uintmax_t) val);
-
-	/* can't take absolute value, as it isn't defined for INTMAX_MIN */
-	do {
-		int mod = val % 10;
-		val /= 10;
-		/* val * 10 + mod == orig val, -10 < mod < 10 */
-		/* we want a negative mod */
-		if (mod > 0) {
-			mod -= 10;
-			val += 1;
-		}
-		/* prepend digit abs(mod) */
-		*(--cur) = (char) ('0' + (-mod));
-	} while (0 != val);
-	*(--cur) = '-';
+	/* absolute value not defined for INTMAX_MIN, but can take absolute
+	 * value of any negative number via twos complement cast to unsigned.
+	 * negative sign is prepended after (now unsigned) value is converted
+	 * to string */
+	uintmax_t uval = val >= 0 ? (uintmax_t)val : ((uintmax_t)~val) + 1;
+	char *cur = utostr(buf_end, uval);
+	if (val < 0) *(--cur) = '-';
 
 	return cur;
 }
diff --git a/src/chunk.c b/src/chunk.c
index 7e7d1062..2bb57c3a 100644
--- a/src/chunk.c
+++ b/src/chunk.c
@@ -458,7 +458,7 @@ static chunk *chunkqueue_get_append_tempfile(chunkqueue *cq) {
 		fd = mkstemp(template->ptr);
 	}
 
-	if (-1 == fd) {
+	if (fd < 0) {
 		buffer_free(template);
 		return NULL;
 	}
@@ -498,7 +498,7 @@ int chunkqueue_append_mem_to_tempfile(server *srv, chunkqueue *dest, const char
 		if (NULL != dst_c
 			&& FILE_CHUNK == dst_c->type
 			&& dst_c->file.is_temp
-			&& -1 != dst_c->file.fd
+			&& dst_c->file.fd >= 0
 			&& 0 == dst_c->offset) {
 			/* ok, take the last chunk for our job */
 
diff --git a/src/http_auth.c b/src/http_auth.c
index 1b621830..8dfe9a6b 100644
--- a/src/http_auth.c
+++ b/src/http_auth.c
@@ -208,6 +208,7 @@ int http_auth_match_rules(server *srv, array *req, const char *username, const c
 	UNUSED(host);
 
 	require = (data_string *)array_get_element(req, "require");
+	if (!require) return -1; /*(should not happen; config is validated at startup)*/
 
 	/* if we get here, the user we got a authed user */
 	if (0 == strcmp(require->value->ptr, "valid-user")) {
@@ -721,6 +722,7 @@ int http_auth_basic_check(server *srv, connection *con, mod_auth_plugin_data *p,
 	data_string *realm;
 
 	realm = (data_string *)array_get_element(req, "realm");
+	if (!realm) return 0; /*(should not happen; config is validated at startup)*/
 
 	username = buffer_init();
 
diff --git a/src/lempar.c b/src/lempar.c
index 9c48fd20..309967c4 100644
--- a/src/lempar.c
+++ b/src/lempar.c
@@ -278,9 +278,10 @@ static void yy_destructor(YYCODETYPE yymajor, YYMINORTYPE *yypminor){
 */
 static int yy_pop_parser_stack(yyParser *pParser){
   YYCODETYPE yymajor;
-  yyStackEntry *yytos = &pParser->yystack[pParser->yyidx];
+  yyStackEntry *yytos;
 
   if( pParser->yyidx<0 ) return 0;
+  yytos = &pParser->yystack[pParser->yyidx];
 #ifndef NDEBUG
   if( yyTraceFILE && pParser->yyidx>=0 ){
     fprintf(yyTraceFILE,"%sPopping %s\n",
@@ -460,10 +461,14 @@ static void yy_reduce(
   ParseARG_FETCH;
   yymsp = &yypParser->yystack[yypParser->yyidx];
 #ifndef NDEBUG
-  if( yyTraceFILE && yyruleno>=0
+  if( yyTraceFILE ) {
+    if (yyruleno>=0
         && (size_t)yyruleno<sizeof(yyRuleName)/sizeof(yyRuleName[0]) ){
-    fprintf(yyTraceFILE, "%sReduce [%s].\n", yyTracePrompt,
-      yyRuleName[yyruleno]);
+      fprintf(yyTraceFILE, "%sReduce [%s].\n", yyTracePrompt,
+        yyRuleName[yyruleno]);
+    } else {
+      return; /*(should not happen)*/
+    }
   }
 #endif /* NDEBUG */
 
diff --git a/src/mod_accesslog.c b/src/mod_accesslog.c
index d7c35dd1..45039ce0 100644
--- a/src/mod_accesslog.c
+++ b/src/mod_accesslog.c
@@ -916,21 +916,23 @@ REQUESTDONE_FUNC(log_access_write) {
 		}
 	}
 
+	if (p->conf.use_syslog) { /* syslog doesn't cache */
+#ifdef HAVE_SYSLOG_H
+		if (!buffer_string_is_empty(b)) {
+			/*(syslog appends a \n on its own)*/
+			syslog(p->conf.syslog_level, "%s", b->ptr);
+			buffer_reset(b);
+		}
+#endif
+		return HANDLER_GO_ON;
+	}
+
 	buffer_append_string_len(b, CONST_STR_LEN("\n"));
 
-	if (p->conf.use_syslog ||  /* syslog doesn't cache */
-	    (!buffer_string_is_empty(p->conf.access_logfile) && p->conf.access_logfile->ptr[0] == '|') || /* pipes don't cache */
+	if ((!buffer_string_is_empty(p->conf.access_logfile) && p->conf.access_logfile->ptr[0] == '|') || /* pipes don't cache */
 	    newts ||
 	    buffer_string_length(b) >= BUFFER_MAX_REUSE_SIZE) {
-		if (p->conf.use_syslog) {
-#ifdef HAVE_SYSLOG_H
-			if (!buffer_string_is_empty(b)) {
-				/* syslog appends a \n on its own */
-				buffer_string_set_length(b, buffer_string_length(b) - 1);
-				syslog(p->conf.syslog_level, "%s", b->ptr);
-			}
-#endif
-		} else if (p->conf.log_access_fd != -1) {
+		if (p->conf.log_access_fd >= 0) {
 			accesslog_write_all(srv, p->conf.access_logfile, p->conf.log_access_fd, CONST_BUF_LEN(b));
 		}
 		buffer_reset(b);
diff --git a/src/mod_auth.c b/src/mod_auth.c
index 9ec76dda..cfadba4b 100644
--- a/src/mod_auth.c
+++ b/src/mod_auth.c
@@ -295,6 +295,9 @@ static handler_t mod_auth_uri_handler(server *srv, connection *con, void *p_d) {
 		con->http_status = 401;
 		con->mode = DIRECT;
 
+		if (!method) return HANDLER_FINISHED;/*(should not happen; config is validated at startup)*/
+		if (!realm) return HANDLER_FINISHED; /*(should not happen; config is validated at startup)*/
+
 		if (0 == strcmp(method->value->ptr, "basic")) {
 			buffer_copy_string_len(p->tmp_buf, CONST_STR_LEN("Basic realm=\""));
 			buffer_append_string_buffer(p->tmp_buf, realm->value);
diff --git a/src/mod_dirlisting.c b/src/mod_dirlisting.c
index 0ac95b2f..b65dbbac 100644
--- a/src/mod_dirlisting.c
+++ b/src/mod_dirlisting.c
@@ -684,9 +684,9 @@ static int http_list_directory(server *srv, connection *con, plugin_data *p, buf
 	name_max = NAME_MAX;
 #endif
 
-	path = malloc(buffer_string_length(dir) + name_max + 1);
+	path = malloc(i + name_max + 1);
 	force_assert(NULL != path);
-	strcpy(path, dir->ptr);
+	memcpy(path, dir->ptr, i+1);
 	path_file = path + i;
 
 	if (NULL == (dp = opendir(path))) {
diff --git a/src/mod_proxy.c b/src/mod_proxy.c
index 0ae53778..97fa1fc8 100644
--- a/src/mod_proxy.c
+++ b/src/mod_proxy.c
@@ -386,7 +386,7 @@ static int proxy_establish_connection(server *srv, handler_ctx *hctx) {
 
 		memset(&proxy_addr_un, 0, sizeof(proxy_addr_un));
 		proxy_addr_un.sun_family = AF_UNIX;
-		strcpy(proxy_addr_un.sun_path, host->host->ptr);
+		memcpy(proxy_addr_un.sun_path, host->host->ptr, buffer_string_length(host->host) + 1);
 		servlen = sizeof(proxy_addr_un);
 		proxy_addr = (struct sockaddr *) &proxy_addr_un;
 	} else
diff --git a/src/network.c b/src/network.c
index 91078eb4..a0b43835 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1099,7 +1099,7 @@ int network_write_chunkqueue(server *srv, connection *con, chunkqueue *cq, off_t
 	 */
 	if (cq->first && cq->first->next) {
 		corked = 1;
-		setsockopt(con->fd, IPPROTO_TCP, TCP_CORK, &corked, sizeof(corked));
+		(void)setsockopt(con->fd, IPPROTO_TCP, TCP_CORK, &corked, sizeof(corked));
 	}
 #endif
 
@@ -1119,7 +1119,7 @@ int network_write_chunkqueue(server *srv, connection *con, chunkqueue *cq, off_t
 #ifdef TCP_CORK
 	if (corked) {
 		corked = 0;
-		setsockopt(con->fd, IPPROTO_TCP, TCP_CORK, &corked, sizeof(corked));
+		(void)setsockopt(con->fd, IPPROTO_TCP, TCP_CORK, &corked, sizeof(corked));
 	}
 #endif
 
diff --git a/src/request.c b/src/request.c
index 6432539c..50898602 100644
--- a/src/request.c
+++ b/src/request.c
@@ -16,7 +16,7 @@ static int request_check_hostname(buffer *host) {
 	enum { DOMAINLABEL, TOPLABEL } stage = TOPLABEL;
 	size_t i;
 	int label_len = 0;
-	size_t host_len;
+	size_t host_len, hostport_len;
 	char *colon;
 	int is_ip = -1; /* -1 don't know yet, 0 no, 1 yes */
 	int level = 0;
@@ -32,8 +32,6 @@ static int request_check_hostname(buffer *host) {
 	 *       port          = *digit
 	 */
 
-	host_len = buffer_string_length(host);
-
 	/* IPv6 adress */
 	if (host->ptr[0] == '[') {
 		char *c = host->ptr + 1;
@@ -70,6 +68,8 @@ static int request_check_hostname(buffer *host) {
 		return 0;
 	}
 
+	hostport_len = host_len = buffer_string_length(host);
+
 	if (NULL != (colon = memchr(host->ptr, ':', host_len))) {
 		char *c = colon + 1;
 
@@ -88,12 +88,11 @@ static int request_check_hostname(buffer *host) {
 	/* if the hostname ends in a "." strip it */
 	if (host->ptr[host_len-1] == '.') {
 		/* shift port info one left */
-		if (NULL != colon) memmove(colon-1, colon, buffer_string_length(host) - host_len);
-		buffer_string_set_length(host, buffer_string_length(host) - 1);
-		host_len -= 1;
+		if (NULL != colon) memmove(colon-1, colon, hostport_len - host_len);
+		buffer_string_set_length(host, --hostport_len);
+		if (--host_len == 0) return -1;
 	}
 
-	if (host_len == 0) return -1;
 
 	/* scan from the right and skip the \0 */
 	for (i = host_len; i-- > 0; ) {