From d7638b9b104864aac48162a42f05f79794697cdb Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 28 Mar 2016 22:24:18 -0400 Subject: [PATCH] fix some warnings reported by static analysis tool iterate over environ via array-index notation with char **ptr on stack (instead of repeatedly re-accessing global 'environ') check getsockname() return values including addrlen [mod_dirlisting] pass buf size into http_list_directory_sizefmt() github: resolves #48 --- src/http-header-glue.c | 3 ++- src/mod_dirlisting.c | 21 +++++++++++---------- src/mod_fastcgi.c | 10 ++++++---- src/mod_scgi.c | 10 ++++++---- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/http-header-glue.c b/src/http-header-glue.c index 992bef0f..5cbb8097 100644 --- a/src/http-header-glue.c +++ b/src/http-header-glue.c @@ -140,7 +140,8 @@ int http_response_redirect_to_directory(server *srv, connection *con) { our_addr_len = sizeof(our_addr); - if (-1 == getsockname(con->fd, &(our_addr.plain), &our_addr_len)) { + if (-1 == getsockname(con->fd, (struct sockaddr *)&our_addr, &our_addr_len) + || our_addr_len > sizeof(our_addr)) { con->http_status = 500; log_error_write(srv, __FILE__, __LINE__, "ss", diff --git a/src/mod_dirlisting.c b/src/mod_dirlisting.c index e373463d..8775736c 100644 --- a/src/mod_dirlisting.c +++ b/src/mod_dirlisting.c @@ -437,11 +437,11 @@ static void http_dirls_sort(dirls_entry_t **ent, int num) { /* buffer must be able to hold "999.9K" * conversion is simple but not perfect */ -static int http_list_directory_sizefmt(char *buf, off_t size) { +static int http_list_directory_sizefmt(char *buf, size_t bufsz, off_t size) { const char unit[] = "KMGTPE"; /* Kilo, Mega, Tera, Peta, Exa */ const char *u = unit - 1; /* u will always increment at least once */ int remain; - char *out = buf; + size_t buflen; if (size < 100) size += 99; @@ -465,14 +465,15 @@ static int http_list_directory_sizefmt(char *buf, off_t size) { u++; } - li_itostrn(out, 4, size); - out += strlen(out); - out[0] = '.'; - out[1] = remain + '0'; - out[2] = *u; - out[3] = '\0'; + li_itostrn(buf, bufsz, size); + buflen = strlen(buf); + if (buflen + 3 >= bufsz) return buflen; + buf[buflen+0] = '.'; + buf[buflen+1] = remain + '0'; + buf[buflen+2] = *u; + buf[buflen+3] = '\0'; - return (out + 3 - buf); + return buflen + 3; } static void http_list_directory_header(server *srv, connection *con, plugin_data *p, buffer *out) { @@ -862,7 +863,7 @@ static int http_list_directory(server *srv, connection *con, plugin_data *p, buf #else strftime(datebuf, sizeof(datebuf), "%Y-%b-%d %H:%M:%S", localtime(&(tmp->mtime))); #endif - http_list_directory_sizefmt(sizebuf, tmp->size); + http_list_directory_sizefmt(sizebuf, sizeof(sizebuf), tmp->size); buffer_append_string_len(out, CONST_STR_LEN("namelen, ENCODING_REL_URI_PART); diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c index 0534207a..64266f97 100644 --- a/src/mod_fastcgi.c +++ b/src/mod_fastcgi.c @@ -1033,11 +1033,12 @@ static int fcgi_spawn_connection(server *srv, } } } else { - for (i = 0; environ[i]; i++) { + char ** const e = environ; + for (i = 0; e[i]; ++i) { char *eq; - if (NULL != (eq = strchr(environ[i], '='))) { - env_add(&env, environ[i], eq - environ[i], eq+1, strlen(eq+1)); + if (NULL != (eq = strchr(e[i], '='))) { + env_add(&env, e[i], eq - e[i], eq+1, strlen(eq+1)); } } } @@ -1926,7 +1927,8 @@ static int fcgi_create_env(server *srv, handler_ctx *hctx, size_t request_id) { /* get the server-side of the connection to the client */ our_addr_len = sizeof(our_addr); - if (-1 == getsockname(con->fd, &(our_addr.plain), &our_addr_len)) { + if (-1 == getsockname(con->fd, (struct sockaddr *)&our_addr, &our_addr_len) + || our_addr_len > sizeof(our_addr)) { s = inet_ntop_cache_get_ip(srv, &(srv_sock->addr)); } else { s = inet_ntop_cache_get_ip(srv, &(our_addr)); diff --git a/src/mod_scgi.c b/src/mod_scgi.c index a08cafbb..22b5c382 100644 --- a/src/mod_scgi.c +++ b/src/mod_scgi.c @@ -825,11 +825,12 @@ static int scgi_spawn_connection(server *srv, } } } else { - for (i = 0; environ[i]; i++) { + char ** const e = environ; + for (i = 0; e[i]; ++i) { char *eq; - if (NULL != (eq = strchr(environ[i], '='))) { - env_add(&env, environ[i], eq - environ[i], eq+1, strlen(eq+1)); + if (NULL != (eq = strchr(e[i], '='))) { + env_add(&env, e[i], eq - e[i], eq+1, strlen(eq+1)); } } } @@ -1576,7 +1577,8 @@ static int scgi_create_env(server *srv, handler_ctx *hctx) { /* get the server-side of the connection to the client */ our_addr_len = sizeof(our_addr); - if (-1 == getsockname(con->fd, &(our_addr.plain), &our_addr_len)) { + if (-1 == getsockname(con->fd, (struct sockaddr *)&our_addr, &our_addr_len) + || our_addr_len > sizeof(our_addr)) { s = inet_ntop_cache_get_ip(srv, &(srv_sock->addr)); } else { s = inet_ntop_cache_get_ip(srv, &(our_addr));