Browse Source

fix errors detected by Coverity Scan

fix potential NULL pointer dereference in mod_deflate.c
remove logically dead code in connection-glue.c
add coverity annotations to see if some issues will be reclassified
personal/stbuehler/mod-csrf
Glenn Strauss 5 years ago
parent
commit
8047c2f448
  1. 7
      src/chunk.c
  2. 4
      src/connections-glue.c
  3. 1
      src/log.c
  4. 2
      src/mod_auth.c
  5. 7
      src/mod_deflate.c
  6. 1
      src/request.c
  7. 1
      src/response.c

7
src/chunk.c

@ -471,6 +471,7 @@ static chunk *chunkqueue_get_append_tempfile(chunkqueue *cq) {
if (-1 != (fd = mkstemp(template->ptr))) break;
}
} else {
/* coverity[secure_temp : FALSE] */
fd = mkstemp(template->ptr);
}
@ -547,6 +548,8 @@ int chunkqueue_append_mem_to_tempfile(server *srv, chunkqueue *dest, const char
return -1;
}
/* (dst_c->file.fd >= 0) */
/* coverity[negative_returns : FALSE] */
written = write(dst_c->file.fd, mem, len);
if ((size_t) written == len) {
@ -585,14 +588,14 @@ int chunkqueue_append_mem_to_tempfile(server *srv, chunkqueue *dest, const char
return -1;
}
}
if (!retry) return -1;
if (!retry) break; /* return -1; */
/* continue; retry */
}
} while (dst_c);
return -1; /*(not reached)*/
return -1;
}
int chunkqueue_steal_with_tempfiles(server *srv, chunkqueue *dest, chunkqueue *src, off_t len) {

4
src/connections-glue.c

@ -206,13 +206,11 @@ static int connection_handle_read_ssl(server *srv, connection *con) {
connection_set_state(srv, con, CON_STATE_ERROR);
return -1;
} else if (len == 0) {
} else { /*(len == 0)*/
con->is_readable = 0;
/* the other end close the connection -> KEEP-ALIVE */
return -2;
} else {
return 0;
}
#else
UNUSED(srv);

1
src/log.c

@ -98,6 +98,7 @@ int openDevNull(int fd) {
dup2(tmpfd, fd);
close(tmpfd);
}
/* coverity[leaked_handle : FALSE] */
return (tmpfd != -1) ? 0 : -1;
}

2
src/mod_auth.c

@ -467,6 +467,7 @@ static handler_t mod_auth_check_basic(server *srv, connection *con, void *p_d, c
username = buffer_init();
b = ds->value;
/* coverity[overflow_sink : FALSE] */
if (!buffer_append_base64_decode(username, b->ptr+sizeof("Basic ")-1, buffer_string_length(b)-(sizeof("Basic ")-1), BASE64_STANDARD)) {
log_error_write(srv, __FILE__, __LINE__, "sb", "decoding base64-string failed", username);
@ -599,6 +600,7 @@ static handler_t mod_auth_check_digest(server *srv, connection *con, void *p_d,
}
b = buffer_init();
/* coverity[overflow_sink : FALSE] */
buffer_copy_string_len(b, ds->value->ptr+sizeof("Digest ")-1, buffer_string_length(ds->value)-(sizeof("Digest ")-1));
/* parse credentials from client */

7
src/mod_deflate.c

@ -684,9 +684,7 @@ static int mod_deflate_file_chunk(server *srv, connection *con, handler_ctx *hct
return -1;
}
#ifdef FD_CLOEXEC
fcntl(c->file.fd, F_SETFD, FD_CLOEXEC);
#endif
fd_close_on_exec(c->file.fd);
}
abs_offset = c->file.start + c->offset;
@ -1075,8 +1073,7 @@ CONNECTION_FUNC(mod_deflate_handle_response_start) {
/* check ETag as is done in http_response_handle_cachable()
* (slightly imperfect (close enough?) match of ETag "000000" to "000000-gzip") */
ds = (data_string *)array_get_element(con->response.headers, "ETag");
if (buffer_string_is_empty(ds->value)) ds = NULL;
if (NULL != ds) {
if (!buffer_string_is_empty(ds->value)) {
etaglen = buffer_string_length(ds->value);
if (etaglen
&& con->http_status < 300 /*(want 2xx only)*/

1
src/request.c

@ -444,6 +444,7 @@ 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.*/
/* coverity[overflow_sink : FALSE] */
buffer_copy_string_len(con->parse_request, con->request.request->ptr + 2, buffer_string_length(con->request.request) - 2);
} else {
/* fill the local request buffer */

1
src/response.c

@ -517,6 +517,7 @@ 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] == '/') {
/* 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 {
buffer_append_string_buffer(con->physical.path, con->physical.rel_path);

Loading…
Cancel
Save