URL: https://github.com/SSSD/sssd/pull/508
Author: lslebodn
 Title: #508: Fix/suppress few gcc8 warnings
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/508/head:pr508
git checkout pr508
From 2da148cbf807916af0282bb552dff6cd171608fd Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 15:34:01 +0100
Subject: [PATCH 1/7] AD: Suppress warning Wincompatible-pointer-types with
 sasl callbacks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SASL use different prototype for callbacks based on id.
However struct sasl_callback_t contains generic callback int (*)(void)
which is not compatible with these callbacks and gcc8 warns about it.

src/providers/ad/ad_init.c:116:23: warning: cast between incompatible
    function types from ‘int (*)(void *, const char *, const char *,
                                 const char **, unsigned int *)’
    to ‘int (*)(void)’ [-Wcast-function-type]
     { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)ad_sasl_getopt, NULL },
                       ^
src/providers/ad/ad_init.c:117:20: warning: cast between incompatible
    function types from ‘int (*)(void *, int,  const char *)’
    to ‘int (*)(void)’ [-Wcast-function-type]
     { SASL_CB_LOG, (sss_sasl_gen_cb_fn)ad_sasl_log, NULL },
                    ^
---
 src/providers/ad/ad_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index e6f3316e2..8c485a7c2 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -113,8 +113,8 @@ static int ad_sasl_log(void *context, int level, const char *message)
 }
 
 static const sasl_callback_t ad_sasl_callbacks[] = {
-    { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)ad_sasl_getopt, NULL },
-    { SASL_CB_LOG, (sss_sasl_gen_cb_fn)ad_sasl_log, NULL },
+    { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)(void *)ad_sasl_getopt, NULL },
+    { SASL_CB_LOG, (sss_sasl_gen_cb_fn)(void *)ad_sasl_log, NULL },
     { SASL_CB_LIST_END, NULL, NULL }
 };
 

From d776e3af8c0e9d36bfbf100d61e76217454031de Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 15:10:14 +0100
Subject: [PATCH 2/7] pysss: Drop unused parameter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

struct PyMethodDef has PyCFunction in definition as ml_meth
and for py_sss_encrypt we needn't use PyCFunctionWithKeywords

src/python/pysss.c:1082:40: warning: cast between incompatible function
    types from ‘PyObject * (*)(PySssPasswordObject *, PyObject *,
                               PyObject *)’
    {aka ‘struct _object * (*)(struct <anonymous> *, struct _object *,
                               struct _object *)’}
    to ‘PyObject * (*)(PyObject *, PyObject *)’
    {aka ‘struct _object * (*)(struct _object *, struct _object *)’}
    [-Wcast-function-type]
     { sss_py_const_p(char, "encrypt"), (PyCFunction) py_sss_encrypt,
                                        ^
---
 src/python/pysss.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/python/pysss.c b/src/python/pysss.c
index dc1c1ff1f..45e20c2c3 100644
--- a/src/python/pysss.c
+++ b/src/python/pysss.c
@@ -963,8 +963,7 @@ PyDoc_STRVAR(py_sss_encrypt__doc__,
 ":param method: The obfuscation method\n\n");
 
 static PyObject *py_sss_encrypt(PySssPasswordObject *self,
-                                PyObject *args,
-                                PyObject *kwds)
+                                PyObject *args)
 {
     char *password = NULL;
     int plen; /* may contain NULL bytes */

From 54e97a4b37e619f6a838354e5e682048edfcb656 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 15:07:13 +0100
Subject: [PATCH 3/7] pysss: Suppress warning Wincompatible-pointer-types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

struct PyMethodDef has PyCFunction in definition as ml_meth
but we are using PyCFunctionWithKeywords

src/python/pysss.c:910:40: warning: initialization of
    ‘PyObject * (*)(PyObject *, PyObject *)’
    {aka ‘struct _object * (*)(struct _object *, struct _object *)’}
    from incompatible pointer type
    ‘PyObject * (*)(PyObject *, PyObject *, PyObject *)’
    {aka ‘struct _object * (*)(struct _object *, struct _object *,
                               struct _object *)’}
    [-Wincompatible-pointer-types]
     { sss_py_const_p(char, "useradd"), (PyCFunction) py_sss_useradd,
                                        ^
src/python/pysss.c:910:40: note: (near initialization for
    ‘sss_local_methods[0].ml_meth’)
---
 src/python/pysss.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/python/pysss.c b/src/python/pysss.c
index 45e20c2c3..16c086a8b 100644
--- a/src/python/pysss.c
+++ b/src/python/pysss.c
@@ -902,22 +902,22 @@ static PyObject *PySssLocalObject_new(PyTypeObject *type,
  * sss.local object methods
  */
 static PyMethodDef sss_local_methods[] = {
-    { sss_py_const_p(char, "useradd"), (PyCFunction) py_sss_useradd,
+    { sss_py_const_p(char, "useradd"), (PyCFunction)(void *) py_sss_useradd,
       METH_KEYWORDS, py_sss_useradd__doc__
     },
-    { sss_py_const_p(char, "userdel"), (PyCFunction) py_sss_userdel,
+    { sss_py_const_p(char, "userdel"), (PyCFunction)(void *) py_sss_userdel,
       METH_KEYWORDS, py_sss_userdel__doc__
     },
-    { sss_py_const_p(char, "usermod"), (PyCFunction) py_sss_usermod,
+    { sss_py_const_p(char, "usermod"), (PyCFunction)(void *) py_sss_usermod,
       METH_KEYWORDS, py_sss_usermod__doc__
     },
-    { sss_py_const_p(char, "groupadd"), (PyCFunction) py_sss_groupadd,
+    { sss_py_const_p(char, "groupadd"), (PyCFunction)(void *) py_sss_groupadd,
       METH_KEYWORDS, py_sss_groupadd__doc__
     },
-    { sss_py_const_p(char, "groupdel"), (PyCFunction) py_sss_groupdel,
+    { sss_py_const_p(char, "groupdel"), (PyCFunction)(void *) py_sss_groupdel,
       METH_KEYWORDS, py_sss_groupdel__doc__
     },
-    { sss_py_const_p(char, "groupmod"), (PyCFunction) py_sss_groupmod,
+    { sss_py_const_p(char, "groupmod"), (PyCFunction)(void *) py_sss_groupmod,
       METH_KEYWORDS, py_sss_groupmod__doc__
     },
     {NULL, NULL, 0, NULL}        /* Sentinel */

From 1403fc21e0647689d80c194939e5a80ea0cc8579 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Sat, 3 Feb 2018 19:04:58 +0000
Subject: [PATCH 4/7] CRYPTO: Suppress warning Wstringop-truncation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

src/util/crypto/nss/nss_sha512crypt.c: In function ‘sha512_crypt_r’:
src/util/crypto/nss/nss_sha512crypt.c:270:10: warning: ‘stpncpy’ output
    truncated copying 3 bytes from a string of length 3
    [-Wstringop-truncation]
     cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE);
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

man gcc says:
           As another example, the following call to "strncpy" results in
           copying to "d" just the characters preceding the terminating NUL,
           without appending the NUL to the end.  Assuming the result of
           "strncpy" is necessarily a NUL-terminated string is a common
           mistake, and so the call is diagnosed.  To avoid the warning when
           the result is not expected to be NUL-terminated, call "memcpy"
           instead.

                   void copy (char *d, const char *s)
                   {
                     strncpy (d, s, strlen (s));
                   }
---
 src/util/crypto/libcrypto/crypto_sha512crypt.c | 2 +-
 src/util/crypto/nss/nss_sha512crypt.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/crypto/libcrypto/crypto_sha512crypt.c b/src/util/crypto/libcrypto/crypto_sha512crypt.c
index b074eee55..5861f34b9 100644
--- a/src/util/crypto/libcrypto/crypto_sha512crypt.c
+++ b/src/util/crypto/libcrypto/crypto_sha512crypt.c
@@ -277,7 +277,7 @@ static int sha512_crypt_r(const char *key,
         goto done;
     }
 
-    cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE);
+    cp = memcpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE);
     buflen -= SALT_PREF_SIZE;
 
     if (rounds_custom) {
diff --git a/src/util/crypto/nss/nss_sha512crypt.c b/src/util/crypto/nss/nss_sha512crypt.c
index 2f1624e63..709cf5196 100644
--- a/src/util/crypto/nss/nss_sha512crypt.c
+++ b/src/util/crypto/nss/nss_sha512crypt.c
@@ -267,7 +267,7 @@ static int sha512_crypt_r(const char *key,
         goto done;
     }
 
-    cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE);
+    cp = memcpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE);
     buflen -= SALT_PREF_SIZE;
 
     if (rounds_custom) {

From e80a149faba3ad45d787a9baf40ac356e928957c Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 16:36:24 +0100
Subject: [PATCH 5/7] INOTIFY: Fix warning Wstringop-truncation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It could not cause any security bug because it is used only with short
names /etc/passwd, /etc/group, /etc/resolv.conf.
And only root could set long names via env variables
SSS_FILES_PASSWD, SSS_FILES_GROUP

  CC       src/util/libsss_files_la-inotify.lo
src/util/inotify.c: In function ‘copy_filenames’:
src/util/inotify.c:390:5: warning: ‘strncpy’ specified bound 4096
    equals destination size [-Wstringop-truncation]
     strncpy(fcopy, filename, sizeof(fcopy));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../sssd/src/util/inotify.c:403:5: warning: ‘strncpy’ specified bound
    4096 equals destination size [-Wstringop-truncation]
     strncpy(fcopy, filename, sizeof(fcopy));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

man gcc says:
           In the following example, the call to "strncpy" specifies the size
           of the destination buffer as the bound.  If the length of the
           source string is equal to or greater than this size the result of
           the copy will not be NUL-terminated.  Therefore, the call is also
           diagnosed.  To avoid the warning, specify "sizeof buf - 1" as the
           bound and set the last element of the buffer to "NUL".

                   void copy (const char *s)
                   {
                     char buf[80];
                     strncpy (buf, s, sizeof buf);
                     ...
                   }
---
 src/util/inotify.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/util/inotify.c b/src/util/inotify.c
index 73dc8a695..2e2dc1a6e 100644
--- a/src/util/inotify.c
+++ b/src/util/inotify.c
@@ -385,10 +385,10 @@ static errno_t copy_filenames(struct snotify_ctx *snctx,
                               const char *filename)
 {
     char *p;
-    char fcopy[PATH_MAX];
+    char fcopy[PATH_MAX + 1];
 
-    strncpy(fcopy, filename, sizeof(fcopy));
-    fcopy[PATH_MAX-1] = '\0';
+    strncpy(fcopy, filename, sizeof(fcopy) - 1);
+    fcopy[PATH_MAX] = '\0';
 
     p = dirname(fcopy);
     if (p == NULL) {
@@ -400,8 +400,8 @@ static errno_t copy_filenames(struct snotify_ctx *snctx,
         return ENOMEM;
     }
 
-    strncpy(fcopy, filename, sizeof(fcopy));
-    fcopy[PATH_MAX-1] = '\0';
+    strncpy(fcopy, filename, sizeof(fcopy) - 1);
+    fcopy[PATH_MAX] = '\0';
 
     p = basename(fcopy);
     if (p == NULL) {

From 4b744d16f7e8695c477eed58bf094be0523cf02e Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 16:16:38 +0100
Subject: [PATCH 6/7] SIFP: Suppress warning Wstringop-truncation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It is a false positive because we allocate 0 initialized function
with size "str_len + 1". Result will be always NUL terminated.

src/lib/sifp/sss_sifp_utils.c: In function ‘sss_sifp_strdup’:
src/lib/sifp/sss_sifp_utils.c:62:5: warning: ‘strncpy’ output
    truncated before terminating nul copying as many bytes from
    a string as its length [-Wstringop-truncation]
     strncpy(result, str, str_len);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/lib/sifp/sss_sifp_utils.c:56:15: note: length computed here
     str_len = strlen(str);
               ^~~~~~~~~~~
---
 src/lib/sifp/sss_sifp_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/sifp/sss_sifp_utils.c b/src/lib/sifp/sss_sifp_utils.c
index dcac71f50..36cbb83a9 100644
--- a/src/lib/sifp/sss_sifp_utils.c
+++ b/src/lib/sifp/sss_sifp_utils.c
@@ -59,7 +59,7 @@ char * sss_sifp_strdup(sss_sifp_ctx *ctx, const char *str)
         return NULL;
     }
 
-    strncpy(result, str, str_len);
+    memcpy(result, str, str_len);
 
     return result;
 }

From 256ed5622071985c12e33ecd73a555caf0a6993c Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Mon, 5 Feb 2018 16:46:16 +0100
Subject: [PATCH 7/7] CLIENT: Fix warning Wstringop-overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It could not cause any problem because all strings were
compile time constants (defines). But it's better to check
that we have enough data for storing NUL terminated string in
nssaddr.sun_path

src/sss_client/common.c: In function ‘sss_cli_check_socket’:
src/sss_client/common.c:544:5: warning: ‘strncpy’ specified bound depends
    on the length of the source argument [-Wstringop-overflow=]
     strncpy(nssaddr.sun_path, socket_name,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             strlen(socket_name) + 1);
             ~~~~~~~~~~~~~~~~~~~~~~~~
src/sss_client/common.c:545:13: note: length computed here
             strlen(socket_name) + 1);
             ^~~~~~~~~~~~~~~~~~~
---
 src/sss_client/common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 17fc8cb7d..67a460705 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -539,10 +539,14 @@ static int sss_cli_open_socket(int *errnop, const char *socket_name, int timeout
     int ret;
     int sd;
 
+    if (sizeof(nssaddr.sun_path) <= strlen(socket_name) + 1) {
+        *errnop = EINVAL;
+        return -1;
+    }
+
     memset(&nssaddr, 0, sizeof(struct sockaddr_un));
     nssaddr.sun_family = AF_UNIX;
-    strncpy(nssaddr.sun_path, socket_name,
-            strlen(socket_name) + 1);
+    strncpy(nssaddr.sun_path, socket_name, sizeof(nssaddr.sun_path));
 
     sd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (sd == -1) {
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to