From: Karol Lewandowski <k.lewandowsk@samsung.com>
Date: Tue, 26 Sep 2017 11:29:32 +0000 (+0200)
Subject: svace fixes: insufficient error handling
X-Git-Tag: accepted/tizen/unified/20180518.120452~6^2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F02%2F152602%2F3;p=platform%2Fcore%2Fsystem%2Fcrash-worker.git

svace fixes: insufficient error handling

This commit brings fixes for folowing SVACE-reported issues:

    * UNCHECKED_FUNC_RES.FREAD: Return value of fread function was compared to zero at crash-stack.c:836, but ferror/feof wasn't called.
      Result was compared with zero at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-stack/crash-stack.c:836

    * HANDLE_LEAK: The handle 'fd' was created at crash-manager.c:508 by calling function 'open' and lost at crash-manager.c:515.
      [acquire] Call of open at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-manager/crash-manager.c:508
      [leaked] leaked at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-manager/crash-manager.c:515

    * DEREF_OF_NULL.CONST: Pointer '&tail->next', which was assigned NULL value at crash-stack.c:642, is dereferenced at crash-stack.c:692.
      [dereference] Variable '&tail->next' is dereferenced at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-stack/crash-stack.c:692
      [null] Assign null at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-stack/crash-stack.c:642

    * RACE.NO_UMASK: Function 'umask(077)' needs to be called before 'mkstemp' at crash-stack.c:964, to prevent a potential race condition vulnerability.
      function call at /home/abuild/rpmbuild/BUILD/crash-worker-1.0.1/src/crash-stack/crash-stack.c:964

Change-Id: Ief6dd93ec8d795fccffbc2d823a6af8fcf63c965
---

diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c
index 1d91d3a..4069aa8 100644
--- a/src/crash-manager/crash-manager.c
+++ b/src/crash-manager/crash-manager.c
@@ -512,6 +512,7 @@ static int lock_dumpdir(void)
 
 	if (flock(fd, LOCK_EX) < 0) {
 		_E("Failed to flock (LOCK)");
+		close(fd);
 		return -1;
 	}
 
diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c
index 30a432c..d0c0c42 100644
--- a/src/crash-stack/crash-stack.c
+++ b/src/crash-stack/crash-stack.c
@@ -46,6 +46,7 @@
 #include <sys/uio.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <assert.h>
 
 #include <elfutils/version.h>
 #include <elfutils/libdwfl.h>
@@ -685,7 +686,9 @@ static struct addr_node *get_addr_list_from_maps(int fd)
 				memcpy(t_node->fpath, STR_ANONY, STR_ANONY_LEN);
 			}
 			t_node->next = NULL;
-			if (head == NULL) {
+			if (head == NULL || tail == NULL) {
+				assert(head == NULL);
+				assert(tail == NULL);
 				head = t_node;
 				tail = t_node;
 			} else {
@@ -833,7 +836,7 @@ static void __print_buffer_info(FILE* bufferfile, FILE *outputfile)
 		fprintf(errfile, "Failed to fseek\n");
 		return;
 	}
-	while ((cnt = fread(buf, sizeof(char), sizeof(buf), bufferfile)) != 0) {
+	while ((cnt = fread(buf, sizeof(char), sizeof(buf), bufferfile)) != 0 && !(feof(bufferfile) || ferror(bufferfile))) {
 		if (cnt != fwrite(buf, sizeof(char), cnt, outputfile))
 			break;
 	}
@@ -961,10 +964,13 @@ int main(int argc, char **argv)
 			tid = pid;
 	}
 
+	mode_t oldmode = umask(0077);
 	if (mkstemp(bufferfile_path) < 0) {
 		fprintf(errfile, "Failed to create buffer file.\n");
 		return errno;
 	}
+	umask(oldmode);
+
 	if ((bufferfile = fopen(bufferfile_path, "w+")) == NULL) {
 		fprintf(errfile, "Failed to open buffer file.\n");
 		return errno;