From 0fff9d8ef54b71b2a7a2c67674ede7a8141835f4 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sat, 30 Dec 2023 16:04:06 +0100 Subject: [PATCH] Fixes after coverity analysis --- CHANGELOG.md | 2 ++ apps/cli/cli_common.c | 2 +- apps/restconf/restconf_methods_get.c | 3 --- apps/snmp/snmp_lib.c | 4 ++-- lib/src/clixon_datastore_write.c | 3 --- lib/src/clixon_err.c | 2 +- lib/src/clixon_netconf_lib.c | 8 ++++---- lib/src/clixon_plugin.c | 16 ++++++++++++---- lib/src/clixon_proc.c | 4 ++-- lib/src/clixon_proto_client.c | 3 +++ lib/src/clixon_stream.c | 11 ++++++++--- lib/src/clixon_xml_changelog.c | 3 +++ lib/src/clixon_yang_type.c | 6 +++++- 13 files changed, 43 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f307ded5..d9bf4d76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Expected: February 2024 ### Minor features +* Made coverity analysis and fixed most of them + * Some were ignored being for generated code (eg lex) or not applicable * Feature: [Add support for -V option to give version](https://github.com/clicon/clixon/issues/472) * All clixon applications added command-line option `-V` for printing version * New ca_version callback for customized version output diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index 767585a3..e0d6f870 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -1596,7 +1596,7 @@ cli_kill_session(clixon_handle h, goto done; } if ((str = cv_string_get(cvec_i(argv, 0))) != NULL){ - if ((ret = parse_uint32(str, &session_id, NULL)) < 0) + if ((ret = parse_uint32(str, &session_id, &reason)) < 0) goto done; if (ret == 0){ cligen_output(stderr, "%s\n", reason); diff --git a/apps/restconf/restconf_methods_get.c b/apps/restconf/restconf_methods_get.c index 4362e940..3e5e30a6 100644 --- a/apps/restconf/restconf_methods_get.c +++ b/apps/restconf/restconf_methods_get.c @@ -372,7 +372,6 @@ api_data_pagination(clixon_handle h, cxobj *xp; cxobj *xpr = NULL; yang_stmt *y = NULL; - cbuf *cbrpc = NULL; int32_t depth = -1; /* Nr of levels to print, -1 is all, 0 is none */ uint32_t limit = 0; uint32_t offset = 0; @@ -568,8 +567,6 @@ api_data_pagination(clixon_handle h, retval = 0; done: clixon_debug(CLIXON_DBG_DEFAULT, "%s retval:%d", __FUNCTION__, retval); - if (cbrpc) - cbuf_free(cbrpc); if (xpath) free(xpath); if (nsc) diff --git a/apps/snmp/snmp_lib.c b/apps/snmp/snmp_lib.c index 048e61eb..b66440d2 100644 --- a/apps/snmp/snmp_lib.c +++ b/apps/snmp/snmp_lib.c @@ -1195,7 +1195,7 @@ snmp_oid2str(oid **oidi, break; } if (cbuf_len(enc)){ - if (cv_string_set(cv, cbuf_get(enc)) < 0){ + if (cv_string_set(cv, cbuf_get(enc)) == NULL){ clixon_err(OE_UNIX, errno, "cv_string_set"); goto done; } @@ -1311,7 +1311,7 @@ snmp_xmlkey2val_oid(cxobj *xentry, break; if (cvk_val){ cv = cvec_i(*cvk_val, i); - if (cv_string_set(cv, xml_body(xi)) < 0){ + if (cv_string_set(cv, xml_body(xi)) == NULL){ clixon_err(OE_UNIX, errno, "cv_string_set"); goto done; } diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index f2cfb032..4c77970e 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -1286,7 +1286,6 @@ xmldb_put(clixon_handle h, int retval = -1; char *dbfile = NULL; FILE *f = NULL; - cbuf *cb = NULL; yang_stmt *yspec; cxobj *x0 = NULL; db_elmnt *de = NULL; @@ -1454,8 +1453,6 @@ xmldb_put(clixon_handle h, xml_nsctx_free(nsc); if (dbfile) free(dbfile); - if (cb) - cbuf_free(cb); if (x0 && clicon_datastore_cache(h) == DATASTORE_NOCACHE) xml_free(x0); return retval; diff --git a/lib/src/clixon_err.c b/lib/src/clixon_err.c index 70fddf14..5610024b 100644 --- a/lib/src/clixon_err.c +++ b/lib/src/clixon_err.c @@ -193,7 +193,7 @@ static char * clixon_strerror1(int err, struct errvec vec[]) { - struct errvec *ev; + struct errvec *ev = NULL; for (ev=vec; ev->ev_err != -1; ev++) if (ev->ev_err == err) diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index 57cf2044..c1110243 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -289,7 +289,7 @@ netconf_missing_attribute_xml(cxobj **xret, char *attr, char *message) { - int retval = -1; + int retval = -1; cxobj *xerr = NULL; char *encstr = NULL; @@ -405,7 +405,7 @@ netconf_bad_attribute_xml(cxobj **xret, char *info, char *message) { - int retval = -1; + int retval = -1; cxobj *xerr = NULL; char *encstr = NULL; @@ -1447,9 +1447,9 @@ netconf_data_not_unique_xml(cxobj **xret, int retval = -1; cg_var *cvi = NULL; cxobj *xerr; - cxobj *xinfo; + cxobj *xinfo = NULL; char *path = NULL; - char *encpath = NULL; + char *encpath = NULL; if (xret == NULL){ clixon_err(OE_NETCONF, EINVAL, "xret is NULL"); diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 06ee20f0..49bc96aa 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -518,6 +518,7 @@ clixon_plugin_start_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Start callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -576,6 +577,7 @@ clixon_plugin_exit_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Exit callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -651,6 +653,7 @@ clixon_plugin_auth_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Auth callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -736,6 +739,7 @@ clixon_plugin_extension_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Extension callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -807,6 +811,7 @@ clixon_plugin_datastore_upgrade_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Datastore upgrade callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -878,6 +883,7 @@ clixon_plugin_yang_mount_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Yang mount callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -945,6 +951,7 @@ clixon_plugin_yang_patch_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Yang patch callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -1005,6 +1012,7 @@ clixon_plugin_netconf_errmsg_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: Netconf err callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -1065,6 +1073,7 @@ clixon_plugin_version_one(clixon_plugin_t *cp, if (clixon_err_category() < 0) clixon_log(h, LOG_WARNING, "%s: Internal error: version callback in plugin: %s returned -1 but did not make a clixon_err call", __FUNCTION__, cp->cp_name); + clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__); goto done; } if (clixon_resource_check(h, &wh, cp->cp_name, __FUNCTION__) < 0) @@ -1240,8 +1249,7 @@ rpc_callback_call(clixon_handle h, goto done; if (rc->rc_callback(h, xe, cbret, arg, rc->rc_arg) < 0){ clixon_debug(CLIXON_DBG_DEFAULT, "%s Error in: %s", __FUNCTION__, rc->rc_name); - if (clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__) < 0) - goto done; + clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__); goto done; } nr++; @@ -1363,12 +1371,12 @@ action_callback_call(clixon_handle h, if ((rc = (rpc_callback_t *)yang_action_cb_get(ya)) != NULL){ do { if (strcmp(rc->rc_name, name) == 0){ + wh = NULL; if (clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__) < 0) goto done; if (rc->rc_callback(h, xa, cbret, arg, rc->rc_arg) < 0){ clixon_debug(CLIXON_DBG_DEFAULT, "%s Error in: %s", __FUNCTION__, rc->rc_name); - if (clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__) < 0) - goto done; + clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__); goto done; } nr++; diff --git a/lib/src/clixon_proc.c b/lib/src/clixon_proc.c index e5fab217..c5fbccc9 100644 --- a/lib/src/clixon_proc.c +++ b/lib/src/clixon_proc.c @@ -327,7 +327,7 @@ clixon_proc_background(clixon_handle h, sigfn_t oldhandler = NULL; sigset_t oset; struct rlimit rlim = {0, }; - struct stat fstat; + struct stat fstat = {0, }; char *flattened; unsigned argc; @@ -340,7 +340,6 @@ clixon_proc_background(clixon_handle h, clixon_err(OE_UNIX, EINVAL, "argv[0] is NULL"); goto quit; } - for (argc = 0; argv[argc] != NULL; ++argc) ; if ((flattened = clicon_strjoin(argc, argv, "', '")) == NULL){ @@ -351,6 +350,7 @@ clixon_proc_background(clixon_handle h, free(flattened); /* Sanity check: program exists */ + // coverity: CID 471757: (#1 of 1): Time of check time of use (TOCTOU)9. fs_check_call: Calling function stat to perform check on argv[0]. See later execvp on filename if (stat(argv[0], &fstat) < 0) { clixon_err(OE_FATAL, errno, "%s", argv[0]); goto quit; diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index 3f117ea3..eccdccbd 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -1101,6 +1101,7 @@ clicon_rpc_get2(clixon_handle h, goto done; xml_sort(xd); /* Ensure attr is first */ *xt = xd; + xd = NULL; } retval = 0; done: @@ -1115,6 +1116,8 @@ clicon_rpc_get2(clixon_handle h, xml_free(xret); if (msg) free(msg); + if (xd) + xml_free(xd); return retval; } diff --git a/lib/src/clixon_stream.c b/lib/src/clixon_stream.c index 3c58afbf..b44aa1b8 100644 --- a/lib/src/clixon_stream.c +++ b/lib/src/clixon_stream.c @@ -170,6 +170,7 @@ int stream_delete_all(clixon_handle h, int force) { + int retval = -1; struct stream_replay *r; struct stream_subscription *ss; event_stream_t *es; @@ -182,8 +183,10 @@ stream_delete_all(clixon_handle h, free(es->es_name); if (es->es_description) free(es->es_description); - while ((ss = es->es_subscription) != NULL) - stream_ss_rm(h, es, ss, force); /* XXX in some cases leaks memory due to DONT clause in stream_ss_rm() */ + while ((ss = es->es_subscription) != NULL){ + if (stream_ss_rm(h, es, ss, force) < 0) + goto done; + } while ((r = es->es_replay) != NULL){ DELQ(r, es->es_replay, struct stream_replay *); if (r->r_xml) @@ -192,7 +195,9 @@ stream_delete_all(clixon_handle h, } free(es); } - return 0; + retval = 0; + done: + return retval; } /*! Return stream definition state in XML supporting RFC 8040 and RFC5277 diff --git a/lib/src/clixon_xml_changelog.c b/lib/src/clixon_xml_changelog.c index d6c61301..404a98ba 100644 --- a/lib/src/clixon_xml_changelog.c +++ b/lib/src/clixon_xml_changelog.c @@ -540,8 +540,11 @@ xml_namespace_vec(clixon_handle h, xvec[i++] = xc; } *vecp = xvec; + xvec = NULL; *veclenp = i; retval = 0; done: + if (xvec) + free(xvec); return retval; } diff --git a/lib/src/clixon_yang_type.c b/lib/src/clixon_yang_type.c index edd782b2..3b86576e 100644 --- a/lib/src/clixon_yang_type.c +++ b/lib/src/clixon_yang_type.c @@ -1479,6 +1479,10 @@ yang_type_get(yang_stmt *ys, yang_stmt *ytype; /* type */ char *type = NULL; + if (yrestype == NULL){ + clixon_err(OE_YANG, EINVAL, "Expected yrestype != NULL"); + goto done; + } if (options) *options = 0x0; /* Find mandatory type */ @@ -1497,7 +1501,7 @@ yang_type_get(yang_stmt *ys, if (yang_type_resolve(ys, ys, ytype, yrestype, options, cvv, patterns, regexps, fraction) < 0) goto done; - if (yrestype && *yrestype == NULL){ + if (*yrestype == NULL){ clixon_err(OE_YANG, 0, "result-type should not be NULL"); goto done; }