Browse Source

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)
personal/stbuehler/mod-csrf-old
Glenn Strauss 5 years ago
parent
commit
72b133f595
  1. 24
      src/buffer.c
  2. 4
      src/chunk.c
  3. 2
      src/http_auth.c
  4. 13
      src/lempar.c
  5. 24
      src/mod_accesslog.c
  6. 3
      src/mod_auth.c
  7. 4
      src/mod_dirlisting.c
  8. 2
      src/mod_proxy.c
  9. 4
      src/network.c
  10. 13
      src/request.c

24
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;
}

4
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 */

2
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();

13
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 */

24
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);

3
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);

4
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))) {

2
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

4
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

13
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; ) {

Loading…
Cancel
Save