svace fixes: insufficient error handling 02/152602/3
authorKarol Lewandowski <k.lewandowsk@samsung.com>
Tue, 26 Sep 2017 11:29:32 +0000 (13:29 +0200)
committerKarol Lewandowski <k.lewandowsk@samsung.com>
Thu, 28 Sep 2017 14:14:53 +0000 (14:14 +0000)
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

src/crash-manager/crash-manager.c
src/crash-stack/crash-stack.c

index 1d91d3a..4069aa8 100644 (file)
@@ -512,6 +512,7 @@ static int lock_dumpdir(void)
 
        if (flock(fd, LOCK_EX) < 0) {
                _E("Failed to flock (LOCK)");
+               close(fd);
                return -1;
        }
 
index 30a432c..d0c0c42 100644 (file)
@@ -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;