From 10c50704c1efb095f3b35de3a535f4ea311cb3e5 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 12 Aug 2016 14:50:00 +0100 Subject: [PATCH] Tidying: coverity issues --- src/OS/os.h-Linux | 4 +++ src/src/deliver.c | 6 ++-- src/src/parse.c | 58 +++++++++++++++++++++++++++++------ src/src/pdkim/pdkim.c | 70 +++++++++++++++++++++++-------------------- src/src/readconf.c | 4 +-- src/src/spool_mbox.c | 13 ++++---- 6 files changed, 100 insertions(+), 55 deletions(-) diff --git a/src/OS/os.h-Linux b/src/OS/os.h-Linux index 05c153e2c..6400e798e 100644 --- a/src/OS/os.h-Linux +++ b/src/OS/os.h-Linux @@ -65,5 +65,9 @@ then change the 0 to 1 in the next block. */ # define LLONG_MAX LONG_LONG_MAX #endif +#if _POSIX_C_SOURCE >= 200809L || _ATFILE_SOUCE +# define EXIM_HAVE_OPENAT +#endif + /* End */ diff --git a/src/src/deliver.c b/src/src/deliver.c index 6fee7be79..dc616a1db 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -1601,7 +1601,7 @@ else force the af_ignore_error flag. This will cause the address to be discarded later (with a log entry). */ - if (sender_address[0] == 0 && message_age >= ignore_bounce_errors_after) + if (!*sender_address && message_age >= ignore_bounce_errors_after) setflag(addr, af_ignore_error); /* Freeze the message if requested, or if this is a bounce message (or other @@ -5578,10 +5578,8 @@ if (deliver_freeze) ignore timer is exceeded. The message will be discarded if this delivery fails. */ - else if (sender_address[0] == 0 && message_age >= ignore_bounce_errors_after) - { + else if (!*sender_address && message_age >= ignore_bounce_errors_after) log_write(0, LOG_MAIN, "Unfrozen by errmsg timer"); - } /* If this is a bounce message, or there's no auto thaw, or we haven't reached the auto thaw time yet, and this delivery is not forced by an admin diff --git a/src/src/parse.c b/src/src/parse.c index d3f382b96..7a072558a 100644 --- a/src/src/parse.c +++ b/src/src/parse.c @@ -1425,7 +1425,7 @@ for (;;) /* Check file name if required */ - if (directory != NULL) + if (directory) { int len = Ustrlen(directory); uschar *p = filename + len; @@ -1437,16 +1437,52 @@ for (;;) return FF_ERROR; } +#ifdef EXIM_HAVE_OPENAT + /* It is necessary to check that every component inside the directory + is NOT a symbolic link, in order to keep the file inside the directory. + This is mighty tedious. We open the directory and openat every component, + with a flag that fails symlinks. */ + + { + int fd = open(directory, O_RDONLY); + if (fd < 0) + { + *error = string_sprintf("failed to open directory %s", directory); + return FF_ERROR; + } + while (*p) + { + uschar temp; + int fd2; + uschar * q = p; + + while (*++p && *p != '/') ; + temp = *p; + *p = '\0'; + + if ((fd2 = openat(fd, q, O_RDONLY|O_NOFOLLOW)) < 0) + { + *error = string_sprintf("failed to open %s (component of included " + "file); could be symbolic link", filename); + return FF_ERROR; + } + close(fd); + fd = fd2; + *p = temp; + } + f = fdopen(fd, "rb"); + } +#else /* It is necessary to check that every component inside the directory is NOT a symbolic link, in order to keep the file inside the directory. This is mighty tedious. It is also not totally foolproof in that it leaves the possibility of a race attack, but I don't know how to do any better. */ - while (*p != 0) + while (*p) { int temp; - while (*(++p) != 0 && *p != '/'); + while (*++p && *p != '/'); temp = *p; *p = 0; if (Ulstat(filename, &statbuf) != 0) @@ -1466,11 +1502,16 @@ for (;;) return FF_ERROR; } } +#endif } - /* Open and stat the file */ +#ifdef EXIM_HAVE_OPENAT + else +#endif + /* Open and stat the file */ + f = Ufopen(filename, "rb"); - if ((f = Ufopen(filename, "rb")) == NULL) + if (!f) { *error = string_open_failed(errno, "included file %s", filename); return FF_INCLUDEFAIL; @@ -1486,7 +1527,7 @@ for (;;) /* If directory was checked, double check that we opened a regular file */ - if (directory != NULL && (statbuf.st_mode & S_IFMT) != S_IFREG) + if (directory && (statbuf.st_mode & S_IFMT) != S_IFREG) { *error = string_sprintf("included file %s is not a regular file in " "the %s directory", filename, directory); @@ -1518,10 +1559,9 @@ for (;;) error, incoming_domain, directory, syntax_errors); if (frc != FF_DELIVERED && frc != FF_NOTDELIVERED) return frc; - if (addr != NULL) + if (addr) { - last = addr; - while (last->next != NULL) { count++; last = last->next; } + for (last = addr; last->next; last = last->next) count++; last->next = *anchor; *anchor = addr; count++; diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index bf2bbba6c..ccd7784f3 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -1014,7 +1014,7 @@ int p; if (!data) pdkim_body_complete(ctx); -for (p = 0; psig; -uschar * headernames = NULL; /* Collected signed header names */ -int hs = 0, hl = 0; /* Check if we must still flush a (partial) header. If that is the case, the message has no body, and we must compute a body hash @@ -1353,6 +1351,8 @@ while (sig) if (ctx->flags & PDKIM_MODE_SIGN) { + uschar * headernames = NULL; /* Collected signed header names */ + int hs = 0, hl = 0; pdkim_stringlist *p; const uschar * l; uschar * s; @@ -1394,7 +1394,6 @@ while (sig) /* Copy headernames to signature struct */ sig->headernames = headernames; - headernames = NULL, hs = hl = 0; /* Create signature header with b= omitted */ sig_hdr = pdkim_create_header(sig, FALSE); @@ -1405,44 +1404,49 @@ while (sig) add the headers to the hash in that order. */ else { - uschar * b = string_copy(sig->headernames); - uschar * p = b; + uschar * p = sig->headernames; uschar * q; pdkim_stringlist * hdrs; - /* clear tags */ - for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next) - hdrs->tag = 0; - - while(1) + if (p) { - if ((q = Ustrchr(p, ':'))) - *q = '\0'; - -/*XXX walk the list of headers in same order as received. */ + /* clear tags */ for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next) - if ( hdrs->tag == 0 - && strncasecmp(hdrs->value, CS p, Ustrlen(p)) == 0 - && (hdrs->value)[Ustrlen(p)] == ':' - ) - { - uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED - ? pdkim_relax_header(hdrs->value, 1) /* cook header for relaxed canon */ - : string_copy(CUS hdrs->value); /* just copy it for simple canon */ + hdrs->tag = 0; - /* Feed header to the hash algorithm */ - exim_sha_update(&hhash_ctx, CUS rh, Ustrlen(rh)); + p = string_copy(p); + while(1) + { + if ((q = Ustrchr(p, ':'))) + *q = '\0'; + + /*XXX walk the list of headers in same order as received. */ + for (hdrs = ctx->headers; hdrs; hdrs = hdrs->next) + if ( hdrs->tag == 0 + && strncasecmp(hdrs->value, CS p, Ustrlen(p)) == 0 + && (hdrs->value)[Ustrlen(p)] == ':' + ) + { + /* cook header for relaxed canon, or just copy it for simple */ + + uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED + ? pdkim_relax_header(hdrs->value, 1) + : string_copy(CUS hdrs->value); + + /* Feed header to the hash algorithm */ + exim_sha_update(&hhash_ctx, CUS rh, Ustrlen(rh)); + + DEBUG(D_acl) pdkim_quoteprint(rh, Ustrlen(rh)); + hdrs->tag = 1; + break; + } - DEBUG(D_acl) pdkim_quoteprint(rh, Ustrlen(rh)); - hdrs->tag = 1; - break; - } + if (!q) break; + p = q+1; + } - if (!q) break; - p = q+1; + sig_hdr = string_copy(sig->rawsig_no_b_val); } - - sig_hdr = string_copy(sig->rawsig_no_b_val); } DEBUG(D_acl) debug_printf( diff --git a/src/src/readconf.c b/src/src/readconf.c index a1591e2a1..c145c7749 100644 --- a/src/src/readconf.c +++ b/src/src/readconf.c @@ -932,10 +932,10 @@ for (;;) save->filename = config_filename; save->lineno = config_lineno; - config_file = Ufopen(ss, "rb"); - if (config_file == NULL) + if (!(config_file = Ufopen(ss, "rb"))) log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN, "failed to open included " "configuration file %s", ss); + config_filename = string_copy(ss); config_lineno = 0; continue; diff --git a/src/src/spool_mbox.c b/src/src/spool_mbox.c index b7ab06127..89691652e 100644 --- a/src/src/spool_mbox.c +++ b/src/src/spool_mbox.c @@ -165,15 +165,14 @@ if (!spool_mbox_ok) } /* get the size of the mbox message and open [message_id].eml file for reading*/ -if (Ustat(mbox_path, &statbuf) != 0 || - (yield = Ufopen(mbox_path,"rb")) == NULL) - { + +if ( !(yield = Ufopen(mbox_path,"rb")) + || fstat(fileno(yield), &statbuf) != 0 + ) log_write(0, LOG_MAIN|LOG_PANIC, "%s", string_open_failed(errno, "scan file %s", mbox_path)); - goto OUT; - } - -*mbox_file_size = statbuf.st_size; +else + *mbox_file_size = statbuf.st_size; OUT: if (data_file) (void)fclose(data_file); -- 2.30.2