Blob Blame History Raw
From 9fa6f3c3ad54d00550f67c8fba0459c22778b0f3 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Fri, 2 Jun 2017 15:08:19 -0400
Subject: [PATCH 11/22] Fix an incorrect error check.

covscan says:
Error: REVERSE_INULL (CWE-476): [#def18]
libsmbios-2.3.3/src/libsmbios_c/memory/memory_linux.c:260: deref_ptr: Directly dereferencing pointer "private_data".
libsmbios-2.3.3/src/libsmbios_c/memory/memory_linux.c:262: check_after_deref: Null-checking "private_data" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

And it's right; private_data->filename has been assigned to between
allocating private_data and checking for success.  Also the free() path
on the error pathway has some invalid reference possibilities in it.

It also says:

Error: FORWARD_NULL (CWE-476): [#def19]
libsmbios-2.3.3/src/libsmbios_c/memory/memory_linux.c:262: var_compare_op: Comparing "private_data->filename" to null implies that "private_data->filename" might be null.
libsmbios-2.3.3/src/libsmbios_c/memory/memory_linux.c:291: var_deref_model: Passing null pointer "private_data->filename" to "strlcat", which dereferences it.
libsmbios-2.3.3/src/libsmbios_c/common/strlcat.c:34:19: var_assign_parm: Assigning: "s" = "src".
libsmbios-2.3.3/src/libsmbios_c/common/strlcat.c:45:9: deref_var_in_call: Function "strlen" dereferences "s" (which is a copy of "src").

And we can just use fn here instead.

So this patch fixes all that up.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 src/libsmbios_c/memory/memory_linux.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/libsmbios_c/memory/memory_linux.c b/src/libsmbios_c/memory/memory_linux.c
index 61532af3dbd..85994e334fe 100644
--- a/src/libsmbios_c/memory/memory_linux.c
+++ b/src/libsmbios_c/memory/memory_linux.c
@@ -234,14 +234,19 @@ static void linux_free(struct memory_access_obj *this)
     struct linux_data *private_data = (struct linux_data *)this->private_data;
     fnprintf("\n");
 
-    free(this->errstring);
-    this->errstring = 0;
+    if (this->errstring)
+        free(this->errstring);
+    this->errstring = NULL;
 
-    free(private_data->filename);
-    private_data->filename = 0;
+    if (private_data)
+    {
+        if (private_data->filename)
+            free(private_data->filename);
+        private_data->filename = NULL;
 
-    free(private_data);
-    this->private_data = 0;
+        free(private_data);
+        this->private_data = NULL;
+    }
 
     this->initialized=0;
 }
@@ -251,22 +256,27 @@ __hidden int init_mem_struct_filename(struct memory_access_obj *m, const char *f
     char *errbuf=0;
     int retval = 0;
     const char *error;
+    struct linux_data *private_data;
 
     fnprintf("\n");
 
     // do allocations
     error = _("There was an allocation failure while trying to construct the memory object. Filename: ");
-    struct linux_data *private_data = calloc(1, sizeof(struct linux_data));
+
+    m->private_data = NULL;
+    m->private_data = private_data = calloc(1, sizeof(struct linux_data));
+    if (!private_data)
+        goto out_fail;
+
     private_data->filename = calloc(1, strlen(fn) + 1);
     m->errstring = calloc(1, ERROR_BUFSIZE);
-    if (!private_data || !private_data->filename || !m->errstring)
+    if (!private_data->filename || !m->errstring)
         goto out_fail;
 
     strcat(private_data->filename, fn);
     private_data->lastMappedOffset = -1;
     private_data->rw = 0;
     private_data->mappingSize = getpagesize(); // must be power of 2, >= getpagesize()
-    m->private_data = private_data;
 
     m->free = linux_free;
     m->read_fn = linux_read_fn;
@@ -287,8 +297,7 @@ out_fail:
     errbuf = memory_get_module_error_buf();
     if (errbuf){
         strlcpy(errbuf, error, ERROR_BUFSIZE);
-
-        strlcat(errbuf, private_data->filename, ERROR_BUFSIZE);
+        strlcat(errbuf, fn, ERROR_BUFSIZE);
         strlcat(errbuf, _("\nThe OS Error string was: "), ERROR_BUFSIZE);
         fixed_strerror(errno, errbuf, ERROR_BUFSIZE);
     }
-- 
2.14.3