From be811502403246414b99f3a8834355c53f6f0511 Mon Sep 17 00:00:00 2001

From: Lukas Slebodnik <lslebodn@redhat.com>

Date: Mon, 6 Jun 2016 18:15:44 +0200

Subject: [PATCH 12/14] TOOLS: Prevent dereference of null pointer

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

VAR_CHECK is called with (var, EOK, ...)

EOK would be returned in case of "var != EOK"

and output argument _attrs would not be initialized.

Therefore there could be dereference of null pointer

after calling function usermod_build_attrs.

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

(cherry picked from commit f9d3aec54d19a771a6eafe09ba6d445cc094bfae)

---

 src/tools/sss_sync_ops.c | 63 +++++++++++++++++++++---------------------------

 1 file changed, 28 insertions(+), 35 deletions(-)

diff --git a/src/tools/sss_sync_ops.c b/src/tools/sss_sync_ops.c

index 7f2e3ea85d5874e3c40f53f327b400e38e430228..a23a0b8c30366d2fb68554bfed184b8fce675e2b 100644

--- a/src/tools/sss_sync_ops.c

+++ b/src/tools/sss_sync_ops.c

@@ -37,13 +37,6 @@

 #define ATTR_NAME_SEP      '='

 #define ATTR_VAL_SEP       ','

-#define VAR_CHECK(var, val, attr, msg) do { \

-        if (var != (val)) { \

-            DEBUG(SSSDBG_CRIT_FAILURE, msg" attribute: %s\n", attr); \

-            return val; \

-        } \

-} while(0)

-

 static int attr_name_val_split(TALLOC_CTX *mem_ctx, const char *nameval,

                                char **_name, char ***_values, int *_nvals)

 {

@@ -200,8 +193,9 @@ static int usermod_build_attrs(TALLOC_CTX *mem_ctx,

                                int lock,

                                struct sysdb_attrs **_attrs)

 {

-    int ret;

+    int ret = EOK;

     struct sysdb_attrs *attrs;

+    const char *attr_name = NULL;

     attrs = sysdb_new_attrs(mem_ctx);

     if (attrs == NULL) {

@@ -209,60 +203,59 @@ static int usermod_build_attrs(TALLOC_CTX *mem_ctx,

     }

     if (shell) {

+        attr_name = SYSDB_SHELL;

         ret = sysdb_attrs_add_string(attrs,

-                                     SYSDB_SHELL,

+                                     attr_name,

                                      shell);

-        VAR_CHECK(ret, EOK, SYSDB_SHELL,

-                  "Could not add attribute to changeset\n");

     }

-    if (home) {

+    if (ret == EOK && home) {

+        attr_name = SYSDB_HOMEDIR;

         ret = sysdb_attrs_add_string(attrs,

-                                     SYSDB_HOMEDIR,

+                                     attr_name,

                                      home);

-        VAR_CHECK(ret, EOK, SYSDB_HOMEDIR,

-                  "Could not add attribute to changeset\n");

     }

-    if (gecos) {

+    if (ret == EOK && gecos) {

+        attr_name = SYSDB_GECOS;

         ret = sysdb_attrs_add_string(attrs,

-                                     SYSDB_GECOS,

+                                     attr_name,

                                      gecos);

-        VAR_CHECK(ret, EOK, SYSDB_GECOS,

-                  "Could not add attribute to changeset\n");

     }

-    if (uid) {

+    if (ret == EOK && uid) {

+        attr_name = SYSDB_UIDNUM;

         ret = sysdb_attrs_add_long(attrs,

-                                   SYSDB_UIDNUM,

+                                   attr_name,

                                    uid);

-        VAR_CHECK(ret, EOK, SYSDB_UIDNUM,

-                  "Could not add attribute to changeset\n");

     }

-    if (gid) {

+    if (ret == EOK && gid) {

+        attr_name = SYSDB_GIDNUM;

         ret = sysdb_attrs_add_long(attrs,

-                                   SYSDB_GIDNUM,

+                                   attr_name,

                                    gid);

-        VAR_CHECK(ret, EOK, SYSDB_GIDNUM,

-                  "Could not add attribute to changeset\n");

     }

-    if (lock == DO_LOCK) {

+    if (ret == EOK && lock == DO_LOCK) {

+        attr_name = SYSDB_DISABLED;

         ret = sysdb_attrs_add_string(attrs,

-                                     SYSDB_DISABLED,

+                                     attr_name,

                                      "true");

-        VAR_CHECK(ret, EOK, SYSDB_DISABLED,

-                  "Could not add attribute to changeset\n");

     }

-    if (lock == DO_UNLOCK) {

+    if (ret == EOK && lock == DO_UNLOCK) {

+        attr_name = SYSDB_DISABLED;

         /* PAM code checks for 'false' value in SYSDB_DISABLED attribute */

         ret = sysdb_attrs_add_string(attrs,

-                                     SYSDB_DISABLED,

+                                     attr_name,

                                      "false");

-        VAR_CHECK(ret, EOK, SYSDB_DISABLED,

-                  "Could not add attribute to changeset\n");

+    }

+

+    if (ret != EOK) {

+        DEBUG(SSSDBG_CRIT_FAILURE,

+              "Could not add attribute [%s] to changeset.\n", attr_name);

+        return ret;

     }

     *_attrs = attrs;

-- 

2.4.11