Ver Fonte

coverity - posix winsync mem leaks, null check, deadcode, null ref, use after free

Fixes the following issues:
13068 Resource leak
In posix_winsync_pre_ds_mod_group_cb(): Leak of memory or pointers to system resources.

13067 Resource leak
In posix_winsync_end_update_cb(): Leak of memory or pointers to system resources

13066 Resource leak
In modGroupMembership(): Leak of memory or pointers to system resources

13065 Resource leak
In dn_in_set(): Leak of memory or pointers to system resources

13064 Resource leak
In dn_in_set(): Leak of memory or pointers to system resources

13063 Dereference after null check
In windows_plugin_add(): Pointer is checked against null but then dereferenced anyway

13062 Explicit null dereferenced
In searchUid(): Dereference of an explicit null value

13061 Logically dead code
In check_account_lock(): Code can never be reached because of a logical contradiction

13059 Use after free
In windows_plugin_cleanup_agmt(): A pointer to freed memory is dereferenced, used as a function argument, or otherwise used
Reviewed by: nkinder (Thanks!)
(cherry picked from commit 1a9b5ffe655c7ec8dd935106376ad8dabe69b561)
Rich Megginson há 13 anos atrás
pai
commit
0c24dfcff9

+ 10 - 4
ldap/servers/plugins/posix-winsync/posix-group-func.c

@@ -86,7 +86,7 @@ searchUid(const char *udn)
             }
             slapi_free_search_results_internal(int_search_pb);
             slapi_pblock_destroy(int_search_pb);
-            if (posix_winsync_config_get_lowercase()) {
+            if (uid && posix_winsync_config_get_lowercase()) {
                 return slapi_dn_ignore_case(uid);
             }
             return uid;
@@ -103,11 +103,15 @@ int
 dn_in_set(const char* uid, char **uids)
 {
     int i;
-    Slapi_DN *sdn_uid = slapi_sdn_new_dn_byval(uid);
-    Slapi_DN *sdn_ul = slapi_sdn_new();
+    Slapi_DN *sdn_uid = NULL;
+    Slapi_DN *sdn_ul = NULL;
 
     if (uids == NULL || uid == NULL)
         return false;
+
+    sdn_uid = slapi_sdn_new_dn_byval(uid);
+    sdn_ul = slapi_sdn_new();
+
     for (i = 0; uids[i]; i++) {
         slapi_sdn_set_dn_byref(sdn_ul, uids[i]);
         if (slapi_sdn_compare(sdn_uid, sdn_ul) == 0) {
@@ -346,7 +350,9 @@ modGroupMembership(Slapi_Entry *entry, Slapi_Mods *smods, int *do_modify)
                                                     "modGroupMembership: add to modlist %s\n", uid);
                                     doModify = true;
                                 }
-                                /*                                slapi_value_free(&v); */
+                                slapi_valueset_free(vs);
+                                slapi_value_init_berval(v, NULL); /* otherwise we will try to free memory we do not own */
+                                slapi_value_free(&v);
                             }
                         }
                     }

+ 5 - 4
ldap/servers/plugins/posix-winsync/posix-winsync.c

@@ -184,8 +184,8 @@ check_account_lock(Slapi_Entry *ds_entry, int *isvirt)
         rc = 1; /* no attr == entry is enabled */
         slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                         "<-- check_account_lock - entry [%s] does not "
-                            "have attribute nsAccountLock - entry %s locked\n",
-                        slapi_entry_get_dn_const(ds_entry), rc ? "is not" : "is");
+                            "have attribute nsAccountLock - entry is not locked\n",
+                        slapi_entry_get_dn_const(ds_entry));
     }
 
     return rc;
@@ -918,7 +918,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla
         slapi_value_init_string(voc, "posixGroup");
         slapi_entry_attr_find(ds_entry, "objectClass", &oc_attr);
         if (slapi_attr_value_find(oc_attr, slapi_value_get_berval(voc)) != 0) {
-            Slapi_ValueSet *oc_vs = slapi_valueset_new();
+            Slapi_ValueSet *oc_vs = NULL;
             Slapi_Value *oc_nv = slapi_value_new();
 
             slapi_attr_get_valueset(oc_attr, &oc_vs);
@@ -933,7 +933,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla
             slapi_value_free(&oc_nv);
             slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                             "_pre_ds_mod_group_cb step\n");
-            /* slapi_valuset_free(oc_vs); */
+            slapi_valueset_free(oc_vs);
             slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                             "_pre_ds_mod_group_cb step\n");
         }
@@ -1292,6 +1292,7 @@ posix_winsync_end_update_cb(void *cbdata, const Slapi_DN *ds_subtree, const Slap
         slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                         "--> posix_winsync_end_update_cb, create task %s\n", dn);
         if (NULL == dn) {
+            slapi_pblock_destroy(pb);
             slapi_log_error(SLAPI_LOG_FATAL, posix_winsync_plugin_name,
                             "posix_winsync_end_update_cb: "
                                 "failed to create task dn: cn=%s,%s,cn=tasks,cn=config\n",

+ 3 - 5
ldap/servers/plugins/replication/windows_private.c

@@ -1246,7 +1246,7 @@ windows_plugin_add(void **theapi, int maxapi)
             }
             elem = PR_NEXT_LINK(elem);
         }
-        if (wpi) { /* was not added - precedence too high */
+        if (elem && wpi) { /* was not added - precedence too high */
             /* just add to end of list */
             PR_INSERT_BEFORE(wpi, elem);
             wpi = NULL; /* owned by list now */
@@ -1372,10 +1372,8 @@ windows_plugin_cleanup_agmt(Repl_Agmt *ra)
 
     while (list && !PR_CLIST_IS_EMPTY(list)) {
         elem = PR_LIST_HEAD(list);
-        if (elem != list) {
-            PR_REMOVE_LINK(elem);
-            slapi_ch_free((void **)&elem);
-        }
+        PR_REMOVE_LINK(elem);
+        slapi_ch_free((void **)&elem);
     }
     slapi_ch_free((void **)&list);
     windows_private_set_api_cookie(ra, NULL);