Blob Blame History Raw
From 5aab1d42916b93e593db632479a800b1de05d169 Mon Sep 17 00:00:00 2001
From: Martin Milata <mmilata@redhat.com>
Date: Mon, 17 Feb 2014 12:28:04 +0100
Subject: [SATYR PATCH 1/6] Fix minor issues found by static analyzers

Avoid possible NULL dereferences of error_msg

Fixes #155.

Signed-off-by: Martin Milata <mmilata@redhat.com>
Signed-off-by: Jakub Filak <jfilak@redhat.com>

Fix insecure string formatting

core_unwind_elfutils.c: In function 'sr_parse_coredump':
core_unwind_elfutils.c:169:13: error: format not a string literal and no format arguments [-Werror=format-security]
             set_error(thread_arg.error_msg);
             ^

Signed-off-by: Martin Milata <mmilata@redhat.com>
---
 lib/Makefile.am             |  2 +-
 lib/core_unwind.c           |  5 +++--
 lib/core_unwind_elfutils.c  | 27 +++++++++++++--------------
 lib/core_unwind_libunwind.c |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index 73fffe2..f798347 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -62,7 +62,7 @@ libsatyr_conv_la_SOURCES = \
 	unstrip.c \
 	utils.c
 
-libsatyr_conv_la_CFLAGS = -Wall -std=gnu99 -D_GNU_SOURCE -I$(top_srcdir)/include $(GLIB_CFLAGS)
+libsatyr_conv_la_CFLAGS = -Wall -Wformat=2 -std=gnu99 -D_GNU_SOURCE -I$(top_srcdir)/include $(GLIB_CFLAGS)
 libsatyr_conv_la_LDFLAGS = $(GLIB_LIBS)
 
 if HAVE_LIBOPCODES
diff --git a/lib/core_unwind.c b/lib/core_unwind.c
index 7910254..8b7cc22 100644
--- a/lib/core_unwind.c
+++ b/lib/core_unwind.c
@@ -227,9 +227,10 @@ open_coredump(const char *elf_file, const char *exe_file, char **error_msg)
     }
     ch->segments = head;
 
-    if (!*error_msg && !head)
+    if (!head)
     {
-        set_error("No segments found in coredump '%s'", elf_file);
+        if (error_msg && !*error_msg)
+            set_error("No segments found in coredump '%s'", elf_file);
         goto fail_dwfl;
     }
 
diff --git a/lib/core_unwind_elfutils.c b/lib/core_unwind_elfutils.c
index a8d8b3f..6b904c7 100644
--- a/lib/core_unwind_elfutils.c
+++ b/lib/core_unwind_elfutils.c
@@ -137,20 +137,20 @@ sr_parse_coredump(const char *core_file,
         *error_msg = NULL;
 
     struct core_handle *ch = open_coredump(core_file, exe_file, error_msg);
-    if (*error_msg)
-        return NULL;
+    if (!ch)
+        goto fail;
 
     if (dwfl_core_file_attach(ch->dwfl, ch->eh) < 0)
     {
         set_error_dwfl("dwfl_core_file_attach");
-        goto fail_destroy_handle;
+        goto fail;
     }
 
     stacktrace = sr_core_stacktrace_new();
     if (!stacktrace)
     {
         set_error("Failed to initialize stacktrace memory");
-        goto fail_destroy_handle;
+        goto fail;
     }
 
     struct thread_callback_arg thread_arg =
@@ -165,11 +165,16 @@ sr_parse_coredump(const char *core_file,
         if (ret == -1)
             set_error_dwfl("dwfl_getthreads");
         else if (ret == DWARF_CB_ABORT)
-            *error_msg = thread_arg.error_msg;
+        {
+            set_error("%s", thread_arg.error_msg);
+            free(thread_arg.error_msg);
+        }
         else
-            *error_msg = sr_strdup("Unknown error in dwfl_getthreads");
+            set_error("Unknown error in dwfl_getthreads");
 
-        goto fail_destroy_trace;
+        sr_core_stacktrace_free(stacktrace);
+        stacktrace = NULL;
+        goto fail;
     }
 
     stacktrace->executable = sr_strdup(exe_file);
@@ -177,13 +182,7 @@ sr_parse_coredump(const char *core_file,
     /* FIXME: is this the best we can do? */
     stacktrace->crash_thread = stacktrace->threads;
 
-fail_destroy_trace:
-    if (*error_msg)
-    {
-        sr_core_stacktrace_free(stacktrace);
-        stacktrace = NULL;
-    }
-fail_destroy_handle:
+fail:
     core_handle_free(ch);
     return stacktrace;
 }
diff --git a/lib/core_unwind_libunwind.c b/lib/core_unwind_libunwind.c
index 966a5b9..b45e2ad 100644
--- a/lib/core_unwind_libunwind.c
+++ b/lib/core_unwind_libunwind.c
@@ -99,7 +99,7 @@ unwind_thread(struct UCD_info *ui,
         }
     }
 
-    if (!error_msg && !trace)
+    if (error_msg && !*error_msg && !trace)
     {
         set_error("No frames found for thread %d", thread_no);
     }
-- 
1.9.3