From 9bc61f16cbb2a14e8225d847bbef24ace21f67c6 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 12 Aug 2017 15:39:12 -0400 Subject: [PATCH] [core] attempt to quiet coverity false positives --- src/buffer.c | 2 +- src/chunk.c | 11 +++++++++++ src/lighttpd-angel.c | 2 ++ src/mod_auth.c | 10 ++++++++++ src/mod_authn_gssapi.c | 4 ++++ src/mod_extforward.c | 7 +++++-- src/mod_rrdtool.c | 34 ++++++++++++---------------------- src/request.c | 7 +++++++ src/response.c | 3 +++ src/server.c | 2 +- 10 files changed, 56 insertions(+), 26 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 673fc0c1..7d95ed50 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -247,7 +247,7 @@ void buffer_append_uint_hex(buffer *b, uintmax_t value) { buf = buffer_string_prepare_append(b, shift); buffer_commit(b, shift); /* will fill below */ - shift <<= 2; /* count bits now */ + shift *= 4; /* count bits now */ while (shift > 0) { shift -= 4; *(buf++) = hex_chars[(value >> shift) & 0x0F]; diff --git a/src/chunk.c b/src/chunk.c index ffcb90a8..8f8c59d8 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -474,10 +474,18 @@ static chunk *chunkqueue_get_append_tempfile(server *srv, chunkqueue *cq) { buffer_append_slash(template); buffer_append_string_len(template, CONST_STR_LEN("lighttpd-upload-XXXXXX")); + #ifdef __COVERITY__ + /* POSIX-2008 requires mkstemp create file with 0600 perms */ + umask(0600); + #endif /* coverity[secure_temp : FALSE] */ if (-1 != (fd = mkstemp(template->ptr))) break; } } else { + #ifdef __COVERITY__ + /* POSIX-2008 requires mkstemp create file with 0600 perms */ + umask(0600); + #endif /* coverity[secure_temp : FALSE] */ fd = mkstemp(template->ptr); } @@ -560,6 +568,9 @@ int chunkqueue_append_mem_to_tempfile(server *srv, chunkqueue *dest, const char if (NULL == dst_c && NULL == (dst_c = chunkqueue_get_append_tempfile(srv, dest))) { return -1; } + #ifdef __COVERITY__ + if (dst_c->file.fd < 0) return -1; + #endif /* (dst_c->file.fd >= 0) */ /* coverity[negative_returns : FALSE] */ diff --git a/src/lighttpd-angel.c b/src/lighttpd-angel.c index 2b88e554..b97f519f 100644 --- a/src/lighttpd-angel.c +++ b/src/lighttpd-angel.c @@ -105,6 +105,8 @@ int main(int argc, char **argv) { argv[0] = BINPATH; + /* intentionally pass argv params */ + /* coverity[tainted_string : FALSE] */ execvp(BINPATH, argv); exit(1); diff --git a/src/mod_auth.c b/src/mod_auth.c index abfdf2d9..33b8babd 100644 --- a/src/mod_auth.c +++ b/src/mod_auth.c @@ -484,6 +484,11 @@ static handler_t mod_auth_check_basic(server *srv, connection *con, void *p_d, c if (0 != strncasecmp(ds->value->ptr, "Basic ", sizeof("Basic ")-1)) { return mod_auth_send_400_bad_request(srv, con); } + #ifdef __COVERITY__ + if (buffer_string_length(ds->value) < sizeof("Basic ")-1) { + return mod_auth_send_400_bad_request(srv, con); + } + #endif username = buffer_init(); @@ -615,6 +620,11 @@ static handler_t mod_auth_check_digest(server *srv, connection *con, void *p_d, if (0 != strncasecmp(ds->value->ptr, "Digest ", sizeof("Digest ")-1)) { return mod_auth_send_400_bad_request(srv, con); } + #ifdef __COVERITY__ + if (buffer_string_length(ds->value) < sizeof("Digest ")-1) { + return mod_auth_send_400_bad_request(srv, con); + } + #endif b = buffer_init(); /* coverity[overflow_sink : FALSE] */ diff --git a/src/mod_authn_gssapi.c b/src/mod_authn_gssapi.c index e61e76a3..fa0a3392 100644 --- a/src/mod_authn_gssapi.c +++ b/src/mod_authn_gssapi.c @@ -218,6 +218,10 @@ static int mod_authn_gssapi_create_krb5_ccache(server *srv, connection *con, plu char * const ccname = kccname->ptr + sizeof("FILE:")-1; const size_t ccnamelen = buffer_string_length(kccname)-(sizeof("FILE:")-1); /*(future: might consider using server.upload-dirs instead of /tmp)*/ + #ifdef __COVERITY__ + /* POSIX-2008 requires mkstemp create file with 0600 perms */ + umask(0600); + #endif /* coverity[secure_temp : FALSE] */ int fd = mkstemp(ccname); if (fd < 0) { diff --git a/src/mod_extforward.c b/src/mod_extforward.c index a3f23697..4be99fc1 100644 --- a/src/mod_extforward.c +++ b/src/mod_extforward.c @@ -1538,8 +1538,11 @@ static int mod_extforward_network_read (server *srv, connection *con, * In the future, might add config switch to enable doing this extra work */ union hap_PROXY_hdr hdr; - int rc; - switch (hap_PROXY_recv(con->fd, &hdr)) { + int rc = hap_PROXY_recv(con->fd, &hdr); + #ifdef __COVERITY__ + __coverity_tainted_data_sanitize__(&hdr); + #endif /*(mod_extforward_hap_PROXY_v*() parse the tainted data)*/ + switch (rc) { case 2: rc = mod_extforward_hap_PROXY_v2(con, &hdr); break; case 1: rc = mod_extforward_hap_PROXY_v1(con, &hdr); break; case 0: return 0; /*(errno == EAGAIN || errno == EWOULDBLOCK)*/ diff --git a/src/mod_rrdtool.c b/src/mod_rrdtool.c index f68587a9..8b80e151 100644 --- a/src/mod_rrdtool.c +++ b/src/mod_rrdtool.c @@ -149,24 +149,21 @@ static ssize_t safe_write(int fd, const void *buf, size_t count) { } /* this assumes we get enough data on a successful read */ -static ssize_t safe_read(int fd, void *buf, size_t count) { +static ssize_t safe_read(int fd, buffer *b) { ssize_t res; - for (;;) { - res = read(fd, buf, count); - if (res >= 0) return res; - switch (errno) { - case EINTR: - continue; - default: - return -1; - } - } + buffer_string_prepare_copy(b, 4095); + + do { + res = read(fd, b->ptr, b->size-1); + } while (-1 == res && errno == EINTR); + + if (res >= 0) buffer_commit(b, res); + return res; } static int mod_rrdtool_create_rrd(server *srv, plugin_data *p, plugin_config *s) { struct stat st; - int r; /* check if DB already exists */ if (0 == stat(s->path_rrd->ptr, &st)) { @@ -211,16 +208,13 @@ static int mod_rrdtool_create_rrd(server *srv, plugin_data *p, plugin_config *s) return HANDLER_ERROR; } - buffer_string_prepare_copy(p->resp, 4095); - if (-1 == (r = safe_read(p->read_fd, p->resp->ptr, p->resp->size - 1))) { + if (-1 == safe_read(p->read_fd, p->resp)) { log_error_write(srv, __FILE__, __LINE__, "ss", "rrdtool-read: failed", strerror(errno)); return HANDLER_ERROR; } - buffer_commit(p->resp, r); - if (p->resp->ptr[0] != 'O' || p->resp->ptr[1] != 'K') { log_error_write(srv, __FILE__, __LINE__, "sbb", @@ -352,7 +346,6 @@ TRIGGER_FUNC(mod_rrd_trigger) { for (i = 0; i < srv->config_context->used; i++) { plugin_config *s = p->config_storage[i]; - int r; if (buffer_string_is_empty(s->path_rrd)) continue; @@ -370,7 +363,7 @@ TRIGGER_FUNC(mod_rrd_trigger) { buffer_append_int(p->cmd, s->requests); buffer_append_string_len(p->cmd, CONST_STR_LEN("\n")); - if (-1 == (r = safe_write(p->write_fd, CONST_BUF_LEN(p->cmd)))) { + if (-1 == safe_write(p->write_fd, CONST_BUF_LEN(p->cmd))) { p->rrdtool_running = 0; log_error_write(srv, __FILE__, __LINE__, "ss", @@ -379,8 +372,7 @@ TRIGGER_FUNC(mod_rrd_trigger) { return HANDLER_ERROR; } - buffer_string_prepare_copy(p->resp, 4095); - if (-1 == (r = safe_read(p->read_fd, p->resp->ptr, p->resp->size - 1))) { + if (-1 == safe_read(p->read_fd, p->resp)) { p->rrdtool_running = 0; log_error_write(srv, __FILE__, __LINE__, "ss", @@ -389,8 +381,6 @@ TRIGGER_FUNC(mod_rrd_trigger) { return HANDLER_ERROR; } - buffer_commit(p->resp, r); - if (p->resp->ptr[0] != 'O' || p->resp->ptr[1] != 'K') { /* don't fail on this error if we just started (graceful restart, the old one might have just updated too) */ diff --git a/src/request.c b/src/request.c index 743589f6..d042d0b8 100644 --- a/src/request.c +++ b/src/request.c @@ -441,6 +441,13 @@ int http_request_parse(server *srv, connection *con) { con->request.request->ptr[1] == '\n') { /* we are in keep-alive and might get \r\n after a previous POST request.*/ + #ifdef __COVERITY__ + if (buffer_string_length(con->request.request) < 2) { + con->keep_alive = 0; + con->http_status = 400; + return 0; + } + #endif /* coverity[overflow_sink : FALSE] */ buffer_copy_string_len(con->parse_request, con->request.request->ptr + 2, buffer_string_length(con->request.request) - 2); } else { diff --git a/src/response.c b/src/response.c index bbdc9f3c..74ab9e18 100644 --- a/src/response.c +++ b/src/response.c @@ -425,6 +425,9 @@ handler_t http_response_prepare(server *srv, connection *con) { buffer_append_slash(con->physical.path); if (!buffer_string_is_empty(con->physical.rel_path) && con->physical.rel_path->ptr[0] == '/') { + #ifdef __COVERITY__ + if (buffer_string_length(con->physical.rel_path) < 1) return HANDLER_ERROR; + #endif /* coverity[overflow_sink : FALSE] */ buffer_append_string_len(con->physical.path, con->physical.rel_path->ptr + 1, buffer_string_length(con->physical.rel_path) - 1); } else { diff --git a/src/server.c b/src/server.c index 639859c7..0b6579ef 100644 --- a/src/server.c +++ b/src/server.c @@ -1170,7 +1170,7 @@ static int server_main (server * const srv, int argc, char **argv) { int devnull; int errfd; do { - /* coverity[resource_leak : FALSE] */ + /* coverity[overwrite_var : FALSE] */ devnull = fdevent_open_devnull(); } while (-1 != devnull && devnull <= STDERR_FILENO); if (-1 == devnull) {