There were many places in the client code where we were duplicating a loop to copy data in from the response buffer. This patch turns those loops into a function for easier maintenance and easier-to-read *readrep() routines.
The netgroup code required the addition of a "temp" string while doing the copying, since the netgroup struct requires a (const char *) and the buffer manipulation was done using (char *). This patch follows "[PATCH] NSS: Validate input string lengths" in my tree but will apply without it.
From 4de3c8267964742c9d6eebdb0f4fd3174f0d1198 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgall...@redhat.com>
Date: Fri, 6 Jan 2012 16:47:50 -0500
Subject: [PATCH] NSS: Add sss_readrep_copy_string
There were many places in the client code where we were
duplicating a loop to copy data in from the response buffer. This
patch turns those loops into a function for easier maintenance and
easier-to-read *readrep() routines.
---
src/sss_client/common.c | 30 ++++++++++++
src/sss_client/nss_group.c | 66 ++++++++------------------
src/sss_client/nss_netgroup.c | 105 ++++++++++++++++-------------------------
src/sss_client/nss_passwd.c | 99 +++++++++++---------------------------
src/sss_client/sss_cli.h | 12 +++++
5 files changed, 131 insertions(+), 181 deletions(-)
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 94d15a529c6be27a94a794556000bf70d749a046..998f7c8ce1ccedbff2ff3a373b9166a794f9c012 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -918,3 +918,33 @@ void sss_pam_lock(void) { return; }
void sss_pam_unlock(void) { return; }
#endif
+
+errno_t sss_readrep_copy_string(const char *in,
+ size_t *offset,
+ size_t *slen,
+ size_t *dlen,
+ char **out,
+ size_t *size)
+{
+ size_t i = 0;
+ while (*slen > *offset && *dlen > 0) {
+ (*out)[i] = in[*offset];
+ if ((*out)[i] == '\0') break;
+ i++;
+ (*offset)++;
+ (*dlen)--;
+ }
+ if (*slen <= *offset) { /* premature end of buf */
+ return EBADMSG;
+ }
+ if (*dlen == 0) { /* not enough memory */
+ return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
+ }
+ (*offset)++;
+ (*dlen)--;
+ if (size) {
+ *size = i;
+ }
+
+ return EOK;
+}
diff --git a/src/sss_client/nss_group.c b/src/sss_client/nss_group.c
index f5e715c86cda2719d3a67f8baf69845c3dbfed30..7e5f79ad5d17d1065fdbd647ed97331e37e18d80 100644
--- a/src/sss_client/nss_group.c
+++ b/src/sss_client/nss_group.c
@@ -78,8 +78,8 @@ struct sss_nss_gr_rep {
static int sss_nss_getgr_readrep(struct sss_nss_gr_rep *pr,
uint8_t *buf, size_t *len)
{
- size_t i, l, slen, ptmem, pad;
- ssize_t dlen;
+ errno_t ret;
+ size_t i, l, slen, ptmem, pad, dlen, glen;
char *sbuf;
uint32_t mem_num;
uint32_t c;
@@ -98,36 +98,19 @@ static int sss_nss_getgr_readrep(struct sss_nss_gr_rep *pr,
pr->result->gr_name = &(pr->buffer[0]);
i = 0;
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->gr_name,
+ NULL);
+ if (ret != EOK) return ret;
pr->result->gr_passwd = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->gr_passwd,
+ NULL);
+ if (ret != EOK) return ret;
/* Make sure pr->buffer[i+pad] is 32 bit aligned */
pad = 0;
@@ -147,22 +130,13 @@ static int sss_nss_getgr_readrep(struct sss_nss_gr_rep *pr,
for (l = 0; l < mem_num; l++) {
pr->result->gr_mem[l] = &(pr->buffer[ptmem]);
- while ((slen > i) && (dlen > 0)) {
- pr->buffer[ptmem] = sbuf[i];
- if (pr->buffer[ptmem] == '\0') break;
- i++;
- dlen--;
- ptmem++;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
- ptmem++;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->gr_mem[l],
+ &glen);
+ if (ret != EOK) return ret;
+
+ ptmem += glen + 1;
}
*len = slen -i;
diff --git a/src/sss_client/nss_netgroup.c b/src/sss_client/nss_netgroup.c
index f63b1135907b3ded6df9fd434395250b1f3944e3..6b9f526030a92287c147c7b9bd624c79d2bc6f42 100644
--- a/src/sss_client/nss_netgroup.c
+++ b/src/sss_client/nss_netgroup.c
@@ -62,9 +62,10 @@ struct sss_nss_netgr_rep {
static int sss_nss_getnetgr_readrep(struct sss_nss_netgr_rep *pr,
uint8_t *buf, size_t *len)
{
+ errno_t ret;
char *sbuf;
- size_t i, slen;
- ssize_t dlen;
+ char *temp;
+ size_t i, slen, dlen;
uint32_t type;
if (*len < 6) {
@@ -84,92 +85,66 @@ static int sss_nss_getnetgr_readrep(struct sss_nss_netgr_rep *pr,
pr->result->type = triple_val;
/* Host value */
- pr->result->val.triple.host = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ temp = &(pr->buffer[i]);
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &temp,
+ NULL);
+ if (ret != EOK) return ret;
/* libc expects NULL instead of empty string */
- if (strlen(pr->result->val.triple.host) == 0) {
+ if (strlen(temp) == 0) {
pr->result->val.triple.host = NULL;
+ } else {
+ pr->result->val.triple.host = temp;
}
/* User value */
- pr->result->val.triple.user = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ temp = &(pr->buffer[i]);
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &temp,
+ NULL);
+ if (ret != EOK) return ret;
/* libc expects NULL instead of empty string */
- if (strlen(pr->result->val.triple.user) == 0) {
+ if (strlen(temp) == 0) {
pr->result->val.triple.user = NULL;
+ } else {
+ pr->result->val.triple.user = temp;
}
/* Domain value */
- pr->result->val.triple.domain = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ temp = &(pr->buffer[i]);
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &temp,
+ NULL);
+ if (ret != EOK) return ret;
/* libc expects NULL instead of empty string */
- if (strlen(pr->result->val.triple.domain) == 0) {
+ if (strlen(temp) == 0) {
pr->result->val.triple.domain = NULL;
+ } else {
+ pr->result->val.triple.domain = temp;
}
break;
+
case SSS_NETGR_REP_GROUP:
pr->result->type = group_val;
- pr->result->val.group = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ temp = &(pr->buffer[i]);
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &temp,
+ NULL);
+ if (ret != EOK) return ret;
+
+ pr->result->val.group = temp;
break;
+
default:
return EBADMSG;
}
diff --git a/src/sss_client/nss_passwd.c b/src/sss_client/nss_passwd.c
index 15de597289183dfa96495ac50b0cf40b83f694db..16124948eb2aea08e382483a5a97fa2ba59ac833 100644
--- a/src/sss_client/nss_passwd.c
+++ b/src/sss_client/nss_passwd.c
@@ -72,6 +72,7 @@ struct sss_nss_pw_rep {
static int sss_nss_getpw_readrep(struct sss_nss_pw_rep *pr,
uint8_t *buf, size_t *len)
{
+ errno_t ret;
size_t i, slen, dlen;
char *sbuf;
uint32_t c;
@@ -91,84 +92,42 @@ static int sss_nss_getpw_readrep(struct sss_nss_pw_rep *pr,
i = 0;
pr->result->pw_name = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->pw_name,
+ NULL);
+ if (ret != EOK) return ret;
pr->result->pw_passwd = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->pw_passwd,
+ NULL);
+ if (ret != EOK) return ret;
pr->result->pw_gecos = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->pw_gecos,
+ NULL);
+ if (ret != EOK) return ret;
+
pr->result->pw_dir = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
- i++;
- dlen--;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->pw_dir,
+ NULL);
+ if (ret != EOK) return ret;
pr->result->pw_shell = &(pr->buffer[i]);
- while (slen > i && dlen > 0) {
- pr->buffer[i] = sbuf[i];
- if (pr->buffer[i] == '\0') break;
- i++;
- dlen--;
- }
- if (slen <= i) { /* premature end of buf */
- return EBADMSG;
- }
- if (dlen <= 0) { /* not enough memory */
- return ERANGE; /* not ENOMEM, ERANGE is what glibc looks for */
- }
-
- *len = slen -i -1;
+ ret = sss_readrep_copy_string(sbuf, &i,
+ &slen, &dlen,
+ &pr->result->pw_shell,
+ NULL);
+ if (ret != EOK) return ret;
+ *len = slen - i;
return 0;
}
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index dc15ac13b22c89ccc56185664d1adc5655fe502c..c6e53e60af3cca68fac6d7b3934584a036870901 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -36,6 +36,11 @@
typedef int errno_t;
#endif
+
+#ifndef EOK
+#define EOK 0
+#endif
+
#define SSS_NSS_PROTOCOL_VERSION 1
#define SSS_PAM_PROTOCOL_VERSION 3
#define SSS_SUDO_PROTOCOL_VERSION 0
@@ -508,4 +513,11 @@ void sss_nss_unlock(void);
void sss_pam_lock(void);
void sss_pam_unlock(void);
+errno_t sss_readrep_copy_string(const char *in,
+ size_t *offset,
+ size_t *slen,
+ size_t *dlen,
+ char **out,
+ size_t *size);
+
#endif /* _SSSCLI_H */
--
1.7.7.5
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
