Covscan fixes

[pemensik]

We checked fstrm library using static analysis and it found some minor issues. Mostly handling of error states with non-freed resources. Each fixed issue is mentioned in commit message.

[pemensik]

There are still few left unfixed.

Error: CPPCHECK_WARNING (CWE-476): [#def1]
fstrm-0.6.0/fstrm/fstrm-private.h:76: error[ctunullpointer]: Null pointer dereference: elems
#   74|   } fs_buf;
#   75|   
#   76|-> VECTOR_GENERATE(fs_bufvec, fs_buf)
#   77|   
#   78|   /* buffer helpers */

Error: CPPCHECK_WARNING (CWE-476): [#def6]
fstrm-0.6.0/libmy/ubuf.h:37: error[ctunullpointer]: Null pointer dereference: elems
#   35|   #include "vector.h"
#   36|   
#   37|-> VECTOR_GENERATE(ubuf, uint8_t)
#   38|   
#   39|   static inline ubuf *

error repeats for elems, out, vec, vec1. I admit I don't see the error or how to fix it.

Error: DEADCODE (CWE-561): [#def5]
fstrm-0.6.0/fstrm/writer.c:360: assignment: Assigning: "iov_max" = "128".
fstrm-0.6.0/fstrm/writer.c:361: const: At condition "iov_max > 1024", the value of "iov_max" must be equal to 128.
fstrm-0.6.0/fstrm/writer.c:361: dead_error_condition: The condition "iov_max > 1024" cannot be true.
fstrm-0.6.0/fstrm/writer.c:362: dead_error_line: Execution cannot reach this statement: "iov_max = 1024;".
#  360|     int iov_max = FSTRM__WRITER_IOVEC_SIZE / 2;
#  361|     if (iov_max > IOV_MAX)
#  362|->       iov_max = IOV_MAX;
#  363|     assert(iov_max > 0);
#  364|   

Is just compile-time check of header define range, no issue with that.

Error: TAINTED_SCALAR (CWE-20): [#def10]
fstrm-0.6.0/src/fstrm_capture.c:1227: tainted_data: Passing tainted expression "**argv" to "parse_args", which uses it as a loop boundary.
fstrm-0.6.0/src/fstrm_capture.c:1227: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
# 1225|   {
# 1226|     /* Parse arguments. */
# 1227|->   if (!parse_args(argc, argv, &g_program_ctx)) {
# 1228|         usage(NULL);
# 1229|         return EXIT_FAILURE;

Error: TAINTED_SCALAR (CWE-20): [#def11]
fstrm-0.6.0/src/fstrm_replay.c:273: tainted_data: Passing tainted expression "**argv" to "parse_args", which uses it as a loop boundary.
fstrm-0.6.0/src/fstrm_replay.c:273: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
#  271|     struct fstrm_writer *w = NULL;
#  272|   
#  273|->   if (!parse_args(argc, argv, &g_program_ctx))
#  274|         usage(NULL);
#  275|   

Again, not sure how to fix this error. Does it check correctly argc?

[d-hat]

The Null pointer dereference warning appears to be in the macro expanded name##append, assuming the elems parameter may be null. I don't think this really needs fixing, as passing null would be an error on the caller's side .. and the impact would be a crash when memcpy tries to read page 0, while the best a fix could do is panic with a more detailed error message. To silence coverity a simple assert(elems != NULL) should do.

TAINTED_SCALAR warnings on argv also seem to be unimportant. Looking down the call chain, what it seems to be referring to is the loop at libmy/argv.c:3166 which loops from **argv until it finds a \0. I would consider this a false positive, since the contents of argv should always be 0-terminated strings.

[reedjc]

I cherry-picked commit cdbebf048184b2962450289a05eac4f1f3574ddb (Fix unsorted printf args) to our next branch for impending release.

[cmikk]

Thank you @pemensik for reporting these issues, and @d-hat for your analysis.

As @reedjc noted, we've cherry-picked the printf correction into our upcoming release, and will take a closer look at the argv.c issues.

[cmikk]

As libmy has its own repository here (and is embedded in various other farsightsec projects), I've moved these commits over to https://github.com/farsightsec/libmy/pull/4 and referenced this PR there for background.

Closing this PR, will work on changes in the core libmy and separately sync the embedded libmy copies.