configure.ac: bump version

Includes static analysis fixes.

Signed-off-by: Alex Jaramillo <alex.v.jaramillo@intel.com>

.github/workflows/ccpp.yml

@@ -8,15 +8,13 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v1
+    - uses: actions/checkout@v4
+      with:
+        ref: ${{ github.event.pull_request.head.sha }}
     - name: install dependencies
-      run: sudo apt-get install libcurl4-gnutls-dev valgrind libelf-dev libdw-dev
-    - name: install check
-      run: wget https://github.com/libcheck/check/releases/download/0.12.0/check-0.12.0.tar.gz &&
-           tar xf check-0.12.0.tar.gz &&
-           pushd check-0.12.0 &&
-           ./configure --prefix=/usr --libdir=/usr/lib/x86_64-linux-gnu &&
-           make -j8 && sudo make install && popd
+      run: |
+        sudo apt-get update
+        sudo apt-get --fix-missing install libcurl4-gnutls-dev valgrind libelf-dev libdw-dev libjson-c-dev check
     - name: autogen
       run: sh autogen.sh
     - name: configure

Makefile.am

@@ -4,7 +4,7 @@ ACLOCAL_AMFLAGS = -I build-aux/m4
 
 EXTRA_DIST = \
 	AUTHORS \
-	LICENSE.LGPL-2.1
+	LICENSE
 
 DISTCHECK_CONFIGURE_FLAGS = \
 	--with-systemdsystemunitdir=$$dc_install_base/$(systemdunitdir) \
@@ -21,7 +21,6 @@ include $(top_srcdir)/src/nica/local.mk
 include $(top_srcdir)/src/probes/local.mk
 include $(top_srcdir)/src/journal/local.mk
 include $(top_srcdir)/tests/local.mk
-include $(top_srcdir)/fuzz/local.mk
 
 release:
 	@git rev-parse v$(PACKAGE_VERSION) &> /dev/null; \

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.69])
-AC_INIT([telemetrics-client], [2.4.0], [https://clearlinux.org/])
+AC_INIT([telemetrics-client], [2.4.2], [https://clearlinux.org/])
 AC_CONFIG_AUX_DIR([build-aux])
 AM_INIT_AUTOMAKE([1.14 -Wall -Werror -Wno-extra-portability foreign subdir-objects])
 AM_SILENT_RULES([yes])

fuzz/.gitignore

@@ -1,3 +0,0 @@
-fuzz_libtelem
-fuzz_config
-fuzz_iorec

fuzz/Makefile

@@ -1,54 +0,0 @@
-bin_PROGRAMS = fuzz_libtelem fuzz_config fuzz_iorec
-
-CC = clang
-CFLAGS = \
-	-g \
-	-I../. \
-	-I../src/. \
-	-DLOCALSTATEDIR=\"/tmp\" \
-	-DDATADIR="" \
-	-DBACKEND_ADDR=\"abcde\"  
-
-# Define the source files for each binary
-fuzz_libtelem_SOURCES = \
-	fuzz_libtelem.c
-
-fuzz_config_SOURCES = \
-	../src/configuration.c \
-	../src/nica/hashmap.c \
-	../src/nica/inifile.c \
-	utils.c \
-	fuzz_config.c
-
-fuzz_iorec_SOURCES = \
-	utils.c \
-	fuzz_iorec.c
-
-fuzz_iorec_CFLAGS = \
-	$(CFLAGS) \
-	-fsanitize=address,fuzzer
-
-fuzz_libtelem_CFLAGS = \
-	$(CFLAGS) \
-	-fsanitize=address,fuzzer \
-	-ltelemetry
-
-fuzz_config_CFLAGS = \
-	$(CFLAGS) \
-	-fsanitize=fuzzer # There's a false positive in configuration.c:189:56
-
-all: $(bin_PROGRAMS)
-
-fuzz_libtelem: $(fuzz_libtelem_SOURCES)
-	$(CC) $(fuzz_libtelem_CFLAGS) -o $@ $^
-
-fuzz_config: $(fuzz_config_SOURCES)
-	$(CC) $(fuzz_config_CFLAGS) -o $@ $^
-
-fuzz_iorec: $(fuzz_iorec_SOURCES)
-	$(CC) $(fuzz_iorec_CFLAGS) -o $@ $^
-
-clean:
-	rm -f $(bin_PROGRAMS) *.o
-
-.PHONY: all clean

fuzz/README.md

@@ -1,21 +0,0 @@
-# Fuzzing telemetrics-client
-
-## Build
-```
-make
-```
-
-## Run
-```
-#fuzz_config
-fuzz_config -close_fd_mask=2
-
-#fuzz_libtelem
-fuzz_libtelem
-```
-
-# Link runs in background
-```
-sudo systemd-run --unit=fuzzing.service -r --uid=$(id -u) \
-    --working-directory=${PWD}/fuzz -E LD_LIBRARY_PATH=${PWD}/src/.libs/ fuzz/<fuzzer_> -max_total_time=86400 -runs=10000000 fuzz/corpus
-```

fuzz/fuzz_config.c

@@ -1,32 +0,0 @@
-#include <configuration.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "utils.h"
-
-void test_configuration(const uint8_t *data, size_t size) {
-
-    char content[16384];
-    char config_file[] = "/tmp/config_fuzzerXXXXXX";
-    static struct configuration config = { { 0 }, { 0 }, { 0 }, false, NULL };
-
-    // Get a null ended string
-    strncpy(content, (const char *)data, size);
-
-    save_data(config_file, content);
-
-    read_config_from_file(config_file, &config);
-    for (int i = 0; i < 6; i++) {
-        if (config.strValues[i] == NULL) {
-            continue;
-        }
-    }
-    unlink(config_file);
-}
-
-int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
-
-    test_configuration(data, size);
-
-    return 0;
-}

fuzz/fuzz_iorec.c

@@ -1,28 +0,0 @@
-#include <configuration.h>
-#include <unistd.h>
-#include <stdint.h>
-#include <stdbool.h>
-#include <string.h>
-
-#include "utils.h"
-
-void test_read_record(const uint8_t *data, size_t size) {
-
-    char content[16384];
-    char record_file[] = "/tmp/record_fuzzerXXXXXX";
-    static struct configuration config = { { 0 }, { 0 }, { 0 }, false, NULL };
-
-    // Get a null ended string
-    strncpy(content, (const char *)data, size);
-
-    save_data(record_file, content);
-
-    // read_record(char *fullpath, char *headers[], char **body, char **cfg_file)
-
-    unlink(record_file);
-}
-
-int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
-    test_read_record(data, size);
-    return 0;
-}

fuzz/fuzz_libtelem.c

@@ -1,73 +0,0 @@
-#include <telemetry.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-
-int test_tm_set_payload(char *payload) {
-    int ret = 0;
-    static uint32_t severity = 1;
-    static uint32_t payload_version = 1;
-    static const char *opt_class = "clearlinux/debug/fuzzing";
-    struct telem_ref *t_ref = NULL;
-
-    if ((ret = tm_create_record(&t_ref, (uint32_t)severity,
-                                (char *)opt_class, payload_version)) < 0) {
-        return ret;
-    }
-    ret = tm_set_payload(t_ref, payload);
-    tm_free_record(t_ref);
-    return ret;
-}
-
-void test_tm_set_config_file(char *path) {
-    /*
-    No need to check returning value because
-    the important part is not it fails, but
-    if this crashes.
-    */
-    tm_set_config_file(path);
-}
-
-int test_tm_set_event_id(char *full_id) {
-    int ret = 0;
-    char *id = NULL;
-    static uint32_t severity = 1;
-    static uint32_t payload_version = 1;
-    static const char *opt_class = "clearlinux/debug/fuzzing";
-    struct telem_ref *t_ref = NULL;
-
-    if ((ret = tm_create_record(&t_ref, (uint32_t)severity,
-                                (char *)opt_class, payload_version)) < 0) {
-        return ret;
-    }
-
-    id = strndup(full_id, 32);
-    ret = tm_set_event_id(t_ref, id);
-    free(id);
-    tm_free_record(t_ref);
-    return ret;
-}
-
-int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
-    int ret = 0;
-    char *payload = NULL;
-
-    /*
-    tm_set_payload expects a null terminated string, this
-    step duplicates data into a null terminated string.
-     */
-    payload = strndup((char *)data, size);
-    if (payload == NULL) {
-        goto end;
-    }
-
-    test_tm_set_payload(payload);
-    test_tm_set_config_file(payload);
-    test_tm_set_event_id(payload);
-
-    free(payload);
-end:
-
-    return ret;
-}

fuzz/local.mk

@@ -1,30 +0,0 @@
-COMPILER = clang
-CLANG_FLAGS = \
-	-g \
-	-fsanitize=address,fuzzer
-SOURCES = fuzz/libtelem_fuzzer.c
-BIN = fuzz/libtelem_fuzzer
-HALF_DAY_SEC = 43200
-FUZZ_FLAGS = \
-	-max_len=16384 \
-	-timeout=10 \
-	-only_ascii=1 \
-	-runs=1000000 \
-	-max_total_time=$(HALF_DAY_SEC)
-
-build_fuzz_libtelem:
-	LD_LIBRARY_PATH=src/.libs $(COMPILER) $(CLANG_FLAGS) -ltelemetry  $(SOURCES) -o $(BIN)
-
-fuzz: build_fuzz_kibtelem
-	$(BIN) fuzz/corpus $(FUZZ_FLAGS)
-
-# vim: filetype=automake tabstop=8 shiftwidth=8 noexpandtab
-
-create a makefile that builds {3 binaries}
-
-{4 binaries}: fuzz_libtelem fuzz_config fuzz_iorec
-
-each one has different compiling flags and SOURCES
-
-
-

fuzz/utils.c

@@ -1,24 +0,0 @@
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-int save_data(char *filename, const char *data) {
-    FILE *temp_file = NULL;
-    int fd;
-
-    fd = mkstemp(filename);
-    if (fd == -1) {
-        return 1;
-    }
-
-    temp_file = fdopen(fd, "w");
-    if (temp_file == NULL) {
-        close(fd);
-        return 1;
-    }
-
-    fprintf(temp_file, "%s", data);
-    fclose(temp_file);
-
-    return 0;
-}

fuzz/utils.h

@@ -1 +0,0 @@
-int save_data(char *filename, const char *data);

src/iorecord.c

@@ -124,8 +124,8 @@ bool read_record(char *fullpath, char *headers[], char **body, char **cfg_file)
         }
         memset(*body, 0, (size_t)size);
 
-        //read rest of file
-        size_t newlen = fread(*body, sizeof(char), (size_t)size, fp);
+        //read rest of file. '-1' ensures it's null terminated
+        size_t newlen = fread(*body, sizeof(char), (size_t)size - 1, fp);
         if (newlen == 0) {
                 telem_perror("Error reading staged file");
                 result = false;

src/journal/journal.c

@@ -421,6 +421,7 @@ static void print_record(char *record_id)
         recordfp = fopen(filepath, "r");
         if (!recordfp) {
                 telem_log(LOG_INFO, "Could not open record %s: %s\n", record_id, strerror(errno));
+                free(filepath);
                 return;
         }
 
@@ -661,7 +662,8 @@ int prune_journal(struct TelemJournal *telem_journal, char *tmp_dir)
                 telem_journal->fptr = fopen(telem_journal->journal_file, "a+");
                 if (!telem_journal->fptr) {
                         telem_log(LOG_ERR, "Error re-opening journal file\n");
-                        return rc;
+                        rc = 1;
+                        goto quit;
                 }
                 // update record count
                 telem_journal->record_count = telem_journal->record_count - count;

src/probes/hello.c

@@ -159,7 +159,7 @@ int main(int argc, char **argv)
                                 break;
                         case 'H':
                                 str = "org.clearlinux/heartbeat/ping";
-                                memcpy(classification, str, strlen(str));
+                                memcpy(classification, str, strlen(str) + 1);
                                 break;
                         case 'l':
                                 payload_options |= PAYLOAD_LOCALE;

src/spool.c

@@ -25,6 +25,7 @@
 #include <limits.h>
 #include <stdbool.h>
 #include <errno.h>
+#include <fcntl.h>
 
 #include "spool.h"
 #include "telempostdaemon.h"
@@ -149,27 +150,37 @@ void process_spooled_record(const char *spool_dir, char *name,
         }
 
         (*records_processed)++;
-        ret = stat(record_name, &buf);
-        if (ret == -1) {
-                telem_perror("Unable to stat record in spool");
-                free(record_name);
-                return;
+        // Use file descriptor to mitigate TOCTOU
+        int fd = open(record_name, O_RDONLY | O_NOFOLLOW);
+        if (fd == -1) {
+                telem_perror("Unable to open record in spool");
+                goto exit;
+        }
+
+        if (fstat(fd, &buf) == -1) {
+                telem_perror("Unable to fstat record in spool");
+                close(fd);
+                goto exit;
         }
 
         /*
-         * If file is a regular file , if uid is diff than process uid
-         * or if mtime is greater than record expiry delete the file
+         * If file is a regular file, if uid is different than process uid,
+         * or if mtime is greater than record expiry, delete the file.
          */
 
         if (record_expiry_config() == -1) {
                 telem_log(LOG_ERR, "Invalid record expiry value\n");
+                close(fd);
                 exit(EXIT_FAILURE);
         }
+
         if (!S_ISREG(buf.st_mode) ||
             (current_time - buf.st_mtime > (record_expiry_config() * 60)) ||
-            (buf.st_uid  != getuid())) {
+            (buf.st_uid != getuid())) {
                 unlink(record_name);
+                close(fd);
         } else if (post_succeeded && *records_sent <= TM_SPOOL_MAX_SEND_RECORDS) {
+                close(fd);
                 transmit_spooled_record(record_name, &post_succeeded, buf.st_size);
 
                 if (!post_succeeded) {
@@ -194,6 +205,7 @@ void process_spooled_record(const char *spool_dir, char *name,
                         }
                 }
         }
+exit:
         free(record_name);
 }
 

src/telempostdaemon.c

@@ -266,7 +266,7 @@ bool post_record_http(char *headers[], char *body, char *cfg)
                        // settings than explicitly requested.
                        // However, report success so the record gets deleted.
                        res = 0;
-                       goto Done;
+                       goto done;
                 }
                 reload_config();
                 telem_debug("DEBUG: override server_addr:%s\n", server_addr_config());
@@ -288,32 +288,35 @@ bool post_record_http(char *headers[], char *body, char *cfg)
                 /* TODO: check if memory needs to be released */
         }
 
-        // Errors for any curl_easy_* functions will store nice error messages
-        // in errorbuf, so send log messages with errorbuf contents
-        curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, errorbuf);
-
-        curl_easy_setopt(curl, CURLOPT_URL, server_addr_config());
-        curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 5L);
-        curl_easy_setopt(curl, CURLOPT_TIMEOUT, 10L);
-        curl_easy_setopt(curl, CURLOPT_POST, 1);
-#ifdef DEBUG
-        curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
-#endif
-
-        curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_callback);
-
         custom_headers = curl_slist_append(custom_headers, tid_header);
         // This should be set by probes/libtelemetry in the future
         custom_headers = curl_slist_append(custom_headers, content);
 
-        curl_easy_setopt(curl, CURLOPT_HTTPHEADER, custom_headers);
-        curl_easy_setopt(curl, CURLOPT_POSTFIELDS, json_body);
-        curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, strlen(json_body));
-        curl_easy_setopt(curl, CURLOPT_USE_SSL, CURLUSESSL_TRY);
+        // Errors for any curl_easy_* functions will store nice error messages
+        // in errorbuf, so send log messages with errorbuf contents
+        if (curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, errorbuf) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_URL, server_addr_config()) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 5L) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_TIMEOUT, 10L) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_POST, 1) != CURLE_OK ||
+#ifdef DEBUG
+            curl_easy_setopt(curl, CURLOPT_VERBOSE, 1) != CURLE_OK ||
+#endif
+            curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_callback) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_HTTPHEADER, custom_headers) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_POSTFIELDS, json_body) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, strlen(json_body)) != CURLE_OK ||
+            curl_easy_setopt(curl, CURLOPT_USE_SSL, CURLUSESSL_TRY) != CURLE_OK) {
+                telem_log(LOG_ERR, "curl_easy_setopt(): Failed to set one or more options\n");
+                goto exit;
+        }
 
         if (strlen(cert_file) > 0) {
                 if (access(cert_file, F_OK) != -1) {
-                        curl_easy_setopt(curl, CURLOPT_CAINFO, cert_file);
+                        if (curl_easy_setopt(curl, CURLOPT_CAINFO, cert_file) != CURLE_OK) {
+                                telem_log(LOG_ERR, "curl_easy_setopt(): Failed to set CAINFO\n");
+                                goto exit;
+                        }
                         telem_log(LOG_INFO, "cafile was set to %s\n", cert_file);
                 }
         }
@@ -344,12 +347,12 @@ bool post_record_http(char *headers[], char *body, char *cfg)
                 telem_log(LOG_INFO, "Record sent successfully\n");
         }
 
+exit:
         curl_slist_free_all(custom_headers);
         curl_easy_cleanup(curl);
-
         curl_global_cleanup();
 
-Done:
+done:
         if (json_body) {
                 free(json_body);
                 json_body = NULL;