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