summaryrefslogtreecommitdiffstats
path: root/server
diff options
context:
space:
mode:
authorSumit Bose <sbose@redhat.com>2009-10-12 12:13:36 +0200
committerStephen Gallagher <sgallagh@redhat.com>2009-10-12 14:18:14 -0400
commit31c1f34db60e47b5501297dfc42f94eb389eedfe (patch)
tree217b13716e3dc9ee618f02f17ee62e5631cd4055 /server
parent0e303315978600c21ad7f9d141d7f4314d5bb035 (diff)
downloadsssd-31c1f34db60e47b5501297dfc42f94eb389eedfe.tar.gz
sssd-31c1f34db60e47b5501297dfc42f94eb389eedfe.tar.xz
sssd-31c1f34db60e47b5501297dfc42f94eb389eedfe.zip
fix a wrong argument to unpack_buffer
- the patch to handle short read introduced a new variable len to store the amount of data read. Instead of using this variable unpack_buffer was called with the old variable ret. Thanks to mnagy@redhat.com for finding this. - this patch also fixes a potential error when the message size is equal to the buffer size.
Diffstat (limited to 'server')
-rw-r--r--server/providers/krb5/krb5_child.c58
1 files changed, 40 insertions, 18 deletions
diff --git a/server/providers/krb5/krb5_child.c b/server/providers/krb5/krb5_child.c
index 7649406f0..9b1be9c95 100644
--- a/server/providers/krb5/krb5_child.c
+++ b/server/providers/krb5/krb5_child.c
@@ -419,49 +419,71 @@ sendresponse:
return EOK;
}
+uint8_t *copy_buffer_and_add_zero(TALLOC_CTX *mem_ctx, const uint8_t *src, size_t len)
+{
+ uint8_t *str;
+
+ str = talloc_size(mem_ctx, len + 1);
+ if (str == NULL) {
+ DEBUG(1, ("talloc_size failed.\n"));
+ return NULL;
+ }
+ memcpy(str, src, len);
+ str[len] = '\0';
+
+ return str;
+}
+
static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd,
char **ccname)
{
size_t p = 0;
uint32_t *len;
- uint8_t *str;
+ if ((p + sizeof(uint32_t)) > size) return EINVAL;
len = ((uint32_t *)(buf+p));
pd->cmd = *len;
p += sizeof(uint32_t);
+ if ((p + sizeof(uint32_t)) > size) return EINVAL;
len = ((uint32_t *)(buf+p));
p += sizeof(uint32_t);
- str = talloc_memdup(pd, buf+p, sizeof(char) * (*len + 1));
- if (str == NULL) return ENOMEM;
- str[*len] = '\0';
- pd->upn = (char *) str;
+
+ if ((p + *len ) > size) return EINVAL;
+ pd->upn = (char *) copy_buffer_and_add_zero(pd, buf+p,
+ sizeof(char) * (*len));
+ if (pd->upn == NULL) return ENOMEM;
p += *len;
+ if ((p + sizeof(uint32_t)) > size) return EINVAL;
len = ((uint32_t *)(buf+p));
p += sizeof(uint32_t);
- str = talloc_memdup(pd, buf+p, sizeof(char) * (*len + 1));
- if (str == NULL) return ENOMEM;
- str[*len] = '\0';
- *ccname = (char *) str;
+
+ if ((p + *len ) > size) return EINVAL;
+ *ccname = (char *) copy_buffer_and_add_zero(pd, buf+p,
+ sizeof(char) * (*len));
+ if (*ccname == NULL) return ENOMEM;
p += *len;
+ if ((p + sizeof(uint32_t)) > size) return EINVAL;
len = ((uint32_t *)(buf+p));
p += sizeof(uint32_t);
- str = talloc_memdup(pd, buf+p, sizeof(char) * (*len + 1));
- if (str == NULL) return ENOMEM;
- str[*len] = '\0';
- pd->authtok = str;
+
+ if ((p + *len) > size) return EINVAL;
+ pd->authtok = copy_buffer_and_add_zero(pd, buf+p, sizeof(char) * (*len));
+ if (pd->authtok == NULL) return ENOMEM;
pd->authtok_size = *len + 1;
p += *len;
if (pd->cmd == SSS_PAM_CHAUTHTOK) {
+ if ((p + sizeof(uint32_t)) > size) return EINVAL;
len = ((uint32_t *)(buf+p));
p += sizeof(uint32_t);
- str = talloc_memdup(pd, buf+p, sizeof(char) * (*len + 1));
- if (str == NULL) return ENOMEM;
- str[*len] = '\0';
- pd->newauthtok = str;
+
+ if ((p + *len) > size) return EINVAL;
+ pd->newauthtok = copy_buffer_and_add_zero(pd, buf+p,
+ sizeof(char) * (*len));
+ if (pd->newauthtok == NULL) return ENOMEM;
pd->newauthtok_size = *len + 1;
p += *len;
} else {
@@ -659,7 +681,7 @@ int main(int argc, char *argv[])
}
close(STDIN_FILENO);
- ret = unpack_buffer(buf, ret, pd, &ccname);
+ ret = unpack_buffer(buf, len, pd, &ccname);
if (ret != EOK) {
DEBUG(1, ("unpack_buffer failed.\n"));
goto fail;