From 94c78224015bfb461a124b994a78c37f8c2a3e34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Sat, 20 Jan 2024 11:51:20 +0100
Subject: [PATCH] Explicitly check sscanf(3) and fscanf(3) return values

Compare the return value of sscanf(3) and fscanf(3) explicitly against
the expected number of parsed items and avoid implicit boolean
conversion.  Such an implicit conversion would treat EOF (-1) the same
as at least one item parsed successfully.

Reported by CodeQL.
---
 Header.c                  |  5 ++---
 Settings.c                |  2 +-
 linux/LinuxMachine.c      | 12 ++++++------
 linux/LinuxProcessTable.c | 10 +++++-----
 linux/Platform.c          |  2 +-
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/Header.c b/Header.c
index 4fee26b41..fa0279da1 100644
--- a/Header.c
+++ b/Header.c
@@ -86,10 +86,9 @@ static void Header_addMeterByName(Header* this, const char* name, MeterModeId mo
    unsigned int param = 0;
    size_t nameLen;
    if (paren) {
-      int ok = sscanf(paren, "(%10u)", &param); // CPUMeter
-      if (!ok) {
+      if (sscanf(paren, "(%10u)", &param) != 1) { // not CPUMeter
          char dynamic[32] = {0};
-         if (sscanf(paren, "(%30s)", dynamic)) { // DynamicMeter
+         if (sscanf(paren, "(%30s)", dynamic) == 1) { // DynamicMeter
             char* end;
             if ((end = strrchr(dynamic, ')')) == NULL)
                return;    // htoprc parse failure
diff --git a/Settings.c b/Settings.c
index a01e2494c..815224bec 100644
--- a/Settings.c
+++ b/Settings.c
@@ -240,7 +240,7 @@ static int toFieldIndex(Hashtable* columns, const char* str) {
    } else {
       // Dynamically-defined columns are always stored by-name.
       char dynamic[32] = {0};
-      if (sscanf(str, "Dynamic(%30s)", dynamic)) {
+      if (sscanf(str, "Dynamic(%30s)", dynamic) == 1) {
          char* end;
          if ((end = strrchr(dynamic, ')')) != NULL) {
             bool success;
diff --git a/linux/LinuxMachine.c b/linux/LinuxMachine.c
index 50d181e16..ff2b605a1 100644
--- a/linux/LinuxMachine.c
+++ b/linux/LinuxMachine.c
@@ -302,8 +302,8 @@ static void LinuxMachine_scanZramInfo(LinuxMachine* this) {
       memory_t orig_data_size = 0;
       memory_t compr_data_size = 0;
 
-      if (!fscanf(disksize_file, "%llu\n", &size) ||
-          !fscanf(mm_stat_file, "    %llu       %llu", &orig_data_size, &compr_data_size)) {
+      if (1 != fscanf(disksize_file, "%llu\n", &size) ||
+          2 != fscanf(mm_stat_file, "    %llu       %llu", &orig_data_size, &compr_data_size)) {
          fclose(disksize_file);
          fclose(mm_stat_file);
          break;
@@ -342,10 +342,10 @@ static void LinuxMachine_scanZfsArcstats(LinuxMachine* this) {
             sscanf(buffer + strlen(label), " %*2u %32llu", variable);          \
             break;                                                             \
          } else (void) 0 /* Require a ";" after the macro use. */
-      #define tryReadFlag(label, variable, flag)                               \
-         if (String_startsWith(buffer, label)) {                               \
-            (flag) = sscanf(buffer + strlen(label), " %*2u %32llu", variable); \
-            break;                                                             \
+      #define tryReadFlag(label, variable, flag)                                      \
+         if (String_startsWith(buffer, label)) {                                      \
+            (flag) = (1 == sscanf(buffer + strlen(label), " %*2u %32llu", variable)); \
+            break;                                                                    \
          } else (void) 0 /* Require a ";" after the macro use. */
 
       switch (buffer[0]) {
diff --git a/linux/LinuxProcessTable.c b/linux/LinuxProcessTable.c
index 83b926192..4386f5c0f 100644
--- a/linux/LinuxProcessTable.c
+++ b/linux/LinuxProcessTable.c
@@ -426,14 +426,14 @@ static bool LinuxProcessTable_readStatusFile(Process* process, openat_arg_t proc
       } else if (String_startsWith(buffer, "voluntary_ctxt_switches:")) {
          unsigned long vctxt;
          int ok = sscanf(buffer, "voluntary_ctxt_switches:\t%lu", &vctxt);
-         if (ok >= 1) {
+         if (ok == 1) {
             ctxt += vctxt;
          }
 
       } else if (String_startsWith(buffer, "nonvoluntary_ctxt_switches:")) {
          unsigned long nvctxt;
          int ok = sscanf(buffer, "nonvoluntary_ctxt_switches:\t%lu", &nvctxt);
-         if (ok >= 1) {
+         if (ok == 1) {
             ctxt += nvctxt;
          }
 
@@ -441,14 +441,14 @@ static bool LinuxProcessTable_readStatusFile(Process* process, openat_arg_t proc
       } else if (String_startsWith(buffer, "VxID:")) {
          int vxid;
          int ok = sscanf(buffer, "VxID:\t%32d", &vxid);
-         if (ok >= 1) {
+         if (ok == 1) {
             lp->vxid = vxid;
          }
 #ifdef HAVE_ANCIENT_VSERVER
       } else if (String_startsWith(buffer, "s_context:")) {
          int vxid;
          int ok = sscanf(buffer, "s_context:\t%32d", &vxid);
-         if (ok >= 1) {
+         if (ok == 1) {
             lp->vxid = vxid;
          }
 #endif /* HAVE_ANCIENT_VSERVER */
@@ -938,7 +938,7 @@ static void LinuxProcessTable_readOomData(LinuxProcess* process, openat_arg_t pr
    if (fgets(buffer, PROC_LINE_LENGTH, file)) {
       unsigned int oom;
       int ok = sscanf(buffer, "%u", &oom);
-      if (ok >= 1) {
+      if (ok == 1) {
          process->oom = oom;
       }
    }
diff --git a/linux/Platform.c b/linux/Platform.c
index 8dc8bb595..af81a694c 100644
--- a/linux/Platform.c
+++ b/linux/Platform.c
@@ -261,7 +261,7 @@ int Platform_getUptime(void) {
    if (fd) {
       int n = fscanf(fd, "%64lf", &uptime);
       fclose(fd);
-      if (n <= 0) {
+      if (n != 1) {
          return 0;
       }
    }