From 7b7c7a0533c564c2fcc08a70e651e7df8e2301fb Mon Sep 17 00:00:00 2001 From: "Kenneth D. Merry" Date: Wed, 25 Feb 2015 04:30:23 +0000 Subject: [PATCH] Fix several problems found by Coverity. lib/libmt/mtlib.c: In mt_start_element(), make sure we don't overflow the cur_sb array. CID 1271325 usr.bin/mt/mt.c: In main(), bzero the mt_com structure so that we aren't using any uninitialized stack variables. CID 1271319 In mt_param(), only allow one -s and one -p argument. This will prevent a memory leak caused by overwriting the param_name and/or param_value variables. CID 1271320 and CID 1271322 To make things simpler in mt_param(), make sure there there is only one exit path for the function. Make sure the arguments are explicitly freed. Sponsored by: Spectra Logic Pointed out by: emaste MFC after: 1 month --- lib/libmt/mtlib.c | 2 +- usr.bin/mt/mt.c | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/libmt/mtlib.c b/lib/libmt/mtlib.c index 2329fa6dc2d..ad73381c582 100644 --- a/lib/libmt/mtlib.c +++ b/lib/libmt/mtlib.c @@ -68,7 +68,7 @@ mt_start_element(void *user_data, const char *name, const char **attr) return; mtinfo->level++; - if ((u_int)mtinfo->level > (sizeof(mtinfo->cur_sb) / + if ((u_int)mtinfo->level >= (sizeof(mtinfo->cur_sb) / sizeof(mtinfo->cur_sb[0]))) { mtinfo->error = 1; snprintf(mtinfo->error_str, sizeof(mtinfo->error_str), diff --git a/usr.bin/mt/mt.c b/usr.bin/mt/mt.c index 4199182d88c..31668318fce 100644 --- a/usr.bin/mt/mt.c +++ b/usr.bin/mt/mt.c @@ -212,6 +212,8 @@ main(int argc, char *argv[]) int ch, len, mtfd; const char *p, *tape; + bzero(&mt_com, sizeof(mt_com)); + if ((tape = getenv("TAPE")) == NULL) tape = DEFTAPE; @@ -1333,12 +1335,24 @@ mt_param(int argc, char **argv, int mtfd, char *xml_str, list = 1; break; case 'p': + if (param_name != NULL) { + warnx("Only one paramter name may be " + "specified"); + retval = 1; + goto bailout; + } param_name = strdup(optarg); break; case 'q': quiet = 1; break; case 's': + if (param_value != NULL) { + warnx("Only one paramter value may be " + "specified"); + retval = 1; + goto bailout; + } param_value = strdup(optarg); do_set = 1; break; @@ -1350,12 +1364,16 @@ mt_param(int argc, char **argv, int mtfd, char *xml_str, } } - if ((list + do_set + xml_dump) != 1) - errx(1, "You must specify only one of -s, -l or -x"); + if ((list + do_set + xml_dump) != 1) { + warnx("You must specify only one of -s, -l or -x"); + retval = 1; + goto bailout; + } if (xml_dump != 0) { printf("%s", xml_str); - return (0); + retval = 0; + goto bailout; } if (do_set != 0) { @@ -1367,6 +1385,9 @@ mt_param(int argc, char **argv, int mtfd, char *xml_str, } else if (list != 0) retval = mt_param_list(status_data, param_name, quiet); +bailout: + free(param_name); + free(param_value); return (retval); }