Browse code

clamonacc: Fix crash when deleting directories

ClamOnAcc may crash when a directory tree is deleted while it's being
scanned. This is easy to reproduce by extracting a large tarball in a
watched directory and then deleting the extracted directory before the
scan is complete.

When removing the inotify nodes, the dirname may be NULL causing a
NULL-dereference. It appears that either the addition or removal
somewhere else in the code is leaving behind the inotify node with a
NULL dirname. I was unable to determine where that bug is, but it was
simple enough to fix the crash by adding a NULL-check. I suspect there's
a memory leak as a result, though a test with valgrind couldn't confirm
it because cleanup in the end on shutdown appears to properly clean up
the inotify watch trees.

Fix addresses https://bugzilla.clamav.net/show_bug.cgi?id=12625

Micah Snyder authored on 2020/12/24 15:28:17
Showing 2 changed files
... ...
@@ -536,7 +536,7 @@ int onas_add_listnode(struct onas_lnode *tail, struct onas_lnode *node)
536 536
 /**
537 537
  * @brief Function to remove a listnode based on dirname.
538 538
  */
539
-int onas_rm_listnode(struct onas_lnode *head, const char *dirname)
539
+cl_error_t onas_rm_listnode(struct onas_lnode *head, const char *dirname)
540 540
 {
541 541
     if (!dirname || !head) return CL_ENULLARG;
542 542
 
... ...
@@ -544,19 +544,21 @@ int onas_rm_listnode(struct onas_lnode *head, const char *dirname)
544 544
     size_t n                = strlen(dirname);
545 545
 
546 546
     while ((curr = curr->next)) {
547
-        if (!strncmp(curr->dirname, dirname, n)) {
548
-            struct onas_lnode *tmp = curr->prev;
549
-            tmp->next              = curr->next;
550
-            tmp                    = curr->next;
551
-            tmp->prev              = curr->prev;
552
-
547
+        if (NULL == curr->dirname) {
548
+            logg("*ClamHash: node's directory name is NULL!\n");
549
+            return CL_ERROR;
550
+        } else if (!strncmp(curr->dirname, dirname, n)) {
551
+            if (curr->next != NULL)
552
+                curr->next->prev = curr->prev;
553
+            if (curr->prev != NULL)
554
+                curr->prev->next = curr->next;
553 555
             onas_free_listnode(curr);
554 556
 
555 557
             return CL_SUCCESS;
556 558
         }
557 559
     }
558 560
 
559
-    return -1;
561
+    return CL_ERROR;
560 562
 }
561 563
 
562 564
 /*** Dealing with parent/child relationships in the table. ***/
... ...
@@ -632,7 +634,9 @@ int onas_ht_rm_child(struct onas_ht *ht, const char *prntpath, size_t prntlen, c
632 632
 
633 633
     hnode = elem->data;
634 634
 
635
-    if ((ret = onas_rm_listnode(hnode->childhead, &(childpath[idx])))) return CL_EARG;
635
+    if (CL_SUCCESS != (ret = onas_rm_listnode(hnode->childhead, &(childpath[idx])))) {
636
+        return CL_EARG;
637
+    }
636 638
 
637 639
     return CL_SUCCESS;
638 640
 }
... ...
@@ -104,7 +104,7 @@ void onas_free_hashnode(struct onas_hnode *hnode);
104 104
 
105 105
 void onas_free_listnode(struct onas_lnode *lnode);
106 106
 int onas_add_listnode(struct onas_lnode *tail, struct onas_lnode *node);
107
-int onas_rm_listnode(struct onas_lnode *head, const char *dirname);
107
+cl_error_t onas_rm_listnode(struct onas_lnode *head, const char *dirname);
108 108
 
109 109
 void onas_free_dirlist(struct onas_lnode *head);
110 110