Fix some issues detected by Coverity

* src/connect.c (connect_to_ip): Check return value of setsockopt.
* src/ftp.c (ftp_retrieve_list): Check return value of chmod.
* src/http.c (digest_authentication_encode): Cleanup code.
* src/init.c (setval_internal): Explicitely check comind range.
* src/main.c (main): Explicitely check optarg.
* src/retr.c (retr_rate): Use snprintf instead sprintf,
  (retrieve_from_file): More verbose error message,
  (rotate_backups): Use snprintf instead sprintf, check return
  value of rename().
* src/url.c (mkalldirs): Check return value of unlink().
* src/utils.c (strdupdelim): Explicitely check beg and end for NULL,
  (merge_vecs): Fix sizeof argument to char *,
  (stable_sort): Use malloc instead of alloca.
This commit is contained in:
Tim Rühsen 2016-09-08 09:24:09 +02:00
parent 37a5257c66
commit a78b83b1e9
8 changed files with 58 additions and 37 deletions

View File

@ -331,8 +331,10 @@ connect_to_ip (const ip_address *ip, int port, const char *print)
if (bufsize < 512) if (bufsize < 512)
bufsize = 512; /* avoid pathologically small values */ bufsize = 512; /* avoid pathologically small values */
#ifdef SO_RCVBUF #ifdef SO_RCVBUF
setsockopt (sock, SOL_SOCKET, SO_RCVBUF, if (setsockopt (sock, SOL_SOCKET, SO_RCVBUF,
(void *)&bufsize, (socklen_t)sizeof (bufsize)); (void *) &bufsize, (socklen_t) sizeof (bufsize)))
logprintf (LOG_NOTQUIET, _("setsockopt SO_RCVBUF failed: %s\n"),
strerror (errno));
#endif #endif
/* When we add limit_rate support for writing, which is useful /* When we add limit_rate support for writing, which is useful
for POST, we should also set SO_SNDBUF here. */ for POST, we should also set SO_SNDBUF here. */
@ -471,7 +473,9 @@ bind_local (const ip_address *bind_address, int *port)
return -1; return -1;
#ifdef SO_REUSEADDR #ifdef SO_REUSEADDR
setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, setopt_ptr, setopt_size); if (setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, setopt_ptr, setopt_size))
logprintf (LOG_NOTQUIET, _("setsockopt SO_REUSEADDR failed: %s\n"),
strerror (errno));
#endif #endif
xzero (ss); xzero (ss);

View File

@ -2363,7 +2363,12 @@ Already have correct symlink %s -> %s\n\n"),
(f->type == FT_PLAINFILE) && opt.preserve_perm) (f->type == FT_PLAINFILE) && opt.preserve_perm)
{ {
if (f->perms) if (f->perms)
chmod (actual_target, f->perms); {
if (chmod (actual_target, f->perms))
logprintf (LOG_NOTQUIET,
_("Failed to set permissions for %s.\n"),
actual_target);
}
else else
DEBUGP (("Unrecognized permissions for %s.\n", actual_target)); DEBUGP (("Unrecognized permissions for %s.\n", actual_target));
} }

View File

@ -4625,7 +4625,7 @@ digest_authentication_encode (const char *au, const char *user,
{ "algorithm", &algorithm } { "algorithm", &algorithm }
}; };
char cnonce[16] = ""; char cnonce[16] = "";
char *res; char *res = NULL;
int res_len; int res_len;
size_t res_size; size_t res_size;
param_token name, value; param_token name, value;
@ -4648,28 +4648,21 @@ digest_authentication_encode (const char *au, const char *user,
} }
} }
if (qop != NULL && strcmp (qop,"auth")) if (qop && strcmp (qop, "auth"))
{ {
logprintf (LOG_NOTQUIET, _("Unsupported quality of protection '%s'.\n"), qop); logprintf (LOG_NOTQUIET, _("Unsupported quality of protection '%s'.\n"), qop);
xfree (qop); /* force freeing mem and return */ xfree (qop); /* force freeing mem and continue */
} }
else if (algorithm != NULL && strcmp (algorithm,"MD5") && strcmp (algorithm,"MD5-sess")) else if (algorithm && strcmp (algorithm,"MD5") && strcmp (algorithm,"MD5-sess"))
{ {
logprintf (LOG_NOTQUIET, _("Unsupported algorithm '%s'.\n"), algorithm); logprintf (LOG_NOTQUIET, _("Unsupported algorithm '%s'.\n"), algorithm);
xfree (qop); /* force freeing mem and return */ xfree (algorithm); /* force freeing mem and continue */
} }
if (!realm || !nonce || !user || !passwd || !path || !method) if (!realm || !nonce || !user || !passwd || !path || !method)
{ {
*auth_err = ATTRMISSING; *auth_err = ATTRMISSING;
goto cleanup;
xfree (realm);
xfree (opaque);
xfree (nonce);
xfree (qop);
xfree (algorithm);
return NULL;
} }
/* Calculate the digest value. */ /* Calculate the digest value. */
@ -4791,6 +4784,7 @@ digest_authentication_encode (const char *au, const char *user,
} }
} }
cleanup:
xfree (realm); xfree (realm);
xfree (opaque); xfree (opaque);
xfree (nonce); xfree (nonce);

View File

@ -872,6 +872,10 @@ static bool
setval_internal (int comind, const char *com, const char *val) setval_internal (int comind, const char *com, const char *val)
{ {
assert (0 <= comind && ((size_t) comind) < countof (commands)); assert (0 <= comind && ((size_t) comind) < countof (commands));
if (comind < 0 || comind >= countof (commands))
return NULL;
DEBUGP (("Setting %s (%s) to %s\n", com, commands[comind].name, val)); DEBUGP (("Setting %s (%s) to %s\n", com, commands[comind].name, val));
return commands[comind].action (com, val, commands[comind].place); return commands[comind].action (com, val, commands[comind].place);
} }

View File

@ -1454,6 +1454,7 @@ main (int argc, char **argv)
append_to_log = true; append_to_log = true;
break; break;
case OPT__EXECUTE: case OPT__EXECUTE:
if (optarg) /* check silences static analyzer */
run_command (optarg); run_command (optarg);
break; break;
case OPT__NO: case OPT__NO:

View File

@ -634,7 +634,7 @@ retr_rate (wgint bytes, double secs)
double dlrate = calc_rate (bytes, secs, &units); double dlrate = calc_rate (bytes, secs, &units);
/* Use more digits for smaller numbers (regardless of unit used), /* Use more digits for smaller numbers (regardless of unit used),
e.g. "1022", "247", "12.5", "2.38". */ e.g. "1022", "247", "12.5", "2.38". */
sprintf (res, "%.*f %s", snprintf (res, sizeof(res), "%.*f %s",
dlrate >= 99.95 ? 0 : dlrate >= 9.995 ? 1 : 2, dlrate >= 99.95 ? 0 : dlrate >= 9.995 ? 1 : 2,
dlrate, !opt.report_bps ? rate_names[units]: rate_names_bits[units]); dlrate, !opt.report_bps ? rate_names[units]: rate_names_bits[units]);
@ -1147,7 +1147,7 @@ retrieve_from_file (const char *file, bool html, int *count)
Removing file due to --delete-after in retrieve_from_file():\n")); Removing file due to --delete-after in retrieve_from_file():\n"));
logprintf (LOG_VERBOSE, _("Removing %s.\n"), filename); logprintf (LOG_VERBOSE, _("Removing %s.\n"), filename);
if (unlink (filename)) if (unlink (filename))
logprintf (LOG_NOTQUIET, "unlink: %s\n", strerror (errno)); logprintf (LOG_NOTQUIET, "Failed to unlink %s: (%d) %s\n", filename, errno, strerror (errno));
dt &= ~RETROKF; dt &= ~RETROKF;
} }
@ -1248,8 +1248,8 @@ rotate_backups(const char *fname)
#endif #endif
int maxlen = strlen (fname) + sizeof (SEP) + numdigit (opt.backups) + AVSL; int maxlen = strlen (fname) + sizeof (SEP) + numdigit (opt.backups) + AVSL;
char *from = (char *)alloca (maxlen); char *from = alloca (maxlen);
char *to = (char *)alloca (maxlen); char *to = alloca (maxlen);
struct_stat sb; struct_stat sb;
int i; int i;
@ -1267,17 +1267,21 @@ rotate_backups(const char *fname)
*/ */
if (i == opt.backups) if (i == opt.backups)
{ {
sprintf (to, "%s%s%d%s", fname, SEP, i, AVS); snprintf (to, sizeof(to), "%s%s%d%s", fname, SEP, i, AVS);
delete (to); delete (to);
} }
#endif #endif
sprintf (to, "%s%s%d", fname, SEP, i); snprintf (to, maxlen, "%s%s%d", fname, SEP, i);
sprintf (from, "%s%s%d", fname, SEP, i - 1); snprintf (from, maxlen, "%s%s%d", fname, SEP, i - 1);
rename (from, to); if (rename (from, to))
logprintf (LOG_NOTQUIET, "Failed to rename %s to %s: (%d) %s\n",
from, to, errno, strerror (errno));
} }
sprintf (to, "%s%s%d", fname, SEP, 1); snprintf (to, maxlen, "%s%s%d", fname, SEP, 1);
rename(fname, to); if (rename(fname, to))
logprintf (LOG_NOTQUIET, "Failed to rename %s to %s: (%d) %s\n",
fname, to, errno, strerror (errno));
} }
static bool no_proxy_match (const char *, const char **); static bool no_proxy_match (const char *, const char **);

View File

@ -1275,7 +1275,9 @@ mkalldirs (const char *path)
name exists, we just remove it and create the directory name exists, we just remove it and create the directory
anyway. */ anyway. */
DEBUGP (("Removing %s because of directory danger!\n", t)); DEBUGP (("Removing %s because of directory danger!\n", t));
unlink (t); if (unlink (t))
logprintf (LOG_NOTQUIET, "Failed to unlink %s (%d): %s\n",
t, errno, strerror(errno));
} }
} }
res = make_directory (t); res = make_directory (t);

View File

@ -235,14 +235,20 @@ xstrdup_lower (const char *s)
/* Copy the string formed by two pointers (one on the beginning, other /* Copy the string formed by two pointers (one on the beginning, other
on the char after the last char) to a new, malloc-ed location. on the char after the last char) to a new, malloc-ed location.
0-terminate it. */ 0-terminate it.
If both pointers are NULL, the function returns an empty string. */
char * char *
strdupdelim (const char *beg, const char *end) strdupdelim (const char *beg, const char *end)
{ {
if (beg && beg <= end)
{
char *res = xmalloc (end - beg + 1); char *res = xmalloc (end - beg + 1);
memcpy (res, beg, end - beg); memcpy (res, beg, end - beg);
res[end - beg] = '\0'; res[end - beg] = '\0';
return res; return res;
}
return xstrdup("");
} }
/* Parse a string containing comma-separated elements, and return a /* Parse a string containing comma-separated elements, and return a
@ -1327,7 +1333,7 @@ merge_vecs (char **v1, char **v2)
for (j = 0; v2[j]; j++) for (j = 0; v2[j]; j++)
; ;
/* Reallocate v1. */ /* Reallocate v1. */
v1 = xrealloc (v1, (i + j + 1) * sizeof (char **)); v1 = xrealloc (v1, (i + j + 1) * sizeof (char *));
memcpy (v1 + i, v2, (j + 1) * sizeof (char *)); memcpy (v1 + i, v2, (j + 1) * sizeof (char *));
xfree (v2); xfree (v2);
return v1; return v1;
@ -2425,8 +2431,9 @@ stable_sort (void *base, size_t nmemb, size_t size,
{ {
if (size > 1) if (size > 1)
{ {
void *temp = alloca (nmemb * size * sizeof (void *)); void *temp = malloc (nmemb * size * sizeof (void *));
mergesort_internal (base, temp, size, 0, nmemb - 1, cmpfun); mergesort_internal (base, temp, size, 0, nmemb - 1, cmpfun);
xfree(temp);
} }
} }