diff options
author | Michal Židek <mzidek@redhat.com> | 2015-07-22 16:35:35 +0200 |
---|---|---|
committer | Jakub Hrozek <jhrozek@redhat.com> | 2015-08-31 18:34:26 +0200 |
commit | 9f0bffebd070115ab47a92eadc6890a721c7b78d (patch) | |
tree | 0cef1e564546161bd056993223e2418f140a44a3 /src/providers | |
parent | 11e8f3ecdddf8edd8b1bbe9f41b49ce8b709b92a (diff) | |
download | sssd-9f0bffebd070115ab47a92eadc6890a721c7b78d.tar.gz sssd-9f0bffebd070115ab47a92eadc6890a721c7b78d.tar.xz sssd-9f0bffebd070115ab47a92eadc6890a721c7b78d.zip |
sssd: incorrect checks on length values during packet decoding
https://fedorahosted.org/sssd/ticket/1697
It is safer to isolate the checked (unknown/untrusted)
value on the left hand side in the conditions
to avoid overflows/underflows.
Reviewed-by: Petr Cech <pcech@redhat.com>
Diffstat (limited to 'src/providers')
-rw-r--r-- | src/providers/ad/ad_gpo_child.c | 8 | ||||
-rw-r--r-- | src/providers/ipa/selinux_child.c | 6 | ||||
-rw-r--r-- | src/providers/krb5/krb5_child.c | 10 | ||||
-rw-r--r-- | src/providers/krb5/krb5_child_handler.c | 6 | ||||
-rw-r--r-- | src/providers/ldap/ldap_child.c | 6 | ||||
-rw-r--r-- | src/providers/ldap/sdap_child_helpers.c | 2 |
6 files changed, 19 insertions, 19 deletions
diff --git a/src/providers/ad/ad_gpo_child.c b/src/providers/ad/ad_gpo_child.c index 03951af04..6547f9c05 100644 --- a/src/providers/ad/ad_gpo_child.c +++ b/src/providers/ad/ad_gpo_child.c @@ -69,7 +69,7 @@ unpack_buffer(uint8_t *buf, if (len == 0) { return EINVAL; } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->smb_server = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->smb_server == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_ALL, "smb_server: %s\n", ibuf->smb_server); @@ -82,7 +82,7 @@ unpack_buffer(uint8_t *buf, if (len == 0) { return EINVAL; } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->smb_share = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->smb_share == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_ALL, "smb_share: %s\n", ibuf->smb_share); @@ -95,7 +95,7 @@ unpack_buffer(uint8_t *buf, if (len == 0) { return EINVAL; } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->smb_path = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->smb_path == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_ALL, "smb_path: %s\n", ibuf->smb_path); @@ -108,7 +108,7 @@ unpack_buffer(uint8_t *buf, if (len == 0) { return EINVAL; } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->smb_cse_suffix = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->smb_cse_suffix == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_ALL, "smb_cse_suffix: %s\n", ibuf->smb_cse_suffix); diff --git a/src/providers/ipa/selinux_child.c b/src/providers/ipa/selinux_child.c index 7c5731d66..3a15e7f51 100644 --- a/src/providers/ipa/selinux_child.c +++ b/src/providers/ipa/selinux_child.c @@ -53,7 +53,7 @@ static errno_t unpack_buffer(uint8_t *buf, DEBUG(SSSDBG_TRACE_INTERNAL, "Empty SELinux user, will delete the mapping\n"); } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->seuser = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->seuser == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_INTERNAL, "seuser: %s\n", ibuf->seuser); @@ -69,7 +69,7 @@ static errno_t unpack_buffer(uint8_t *buf, return EINVAL; } } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->mls_range = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->mls_range == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_INTERNAL, "mls_range: %s\n", ibuf->mls_range); @@ -83,7 +83,7 @@ static errno_t unpack_buffer(uint8_t *buf, DEBUG(SSSDBG_CRIT_FAILURE, "No username set!\n"); return EINVAL; } else { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->username = talloc_strndup(ibuf, (char *)(buf + p), len); if (ibuf->username == NULL) return ENOMEM; DEBUG(SSSDBG_TRACE_INTERNAL, "username: %s\n", ibuf->username); diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index e5f48b713..1edf10ab8 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -1808,7 +1808,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, SAFEALIGN_COPY_UINT32_CHECK(&use_enterprise_princ, buf + p, size, &p); kr->use_enterprise_princ = (use_enterprise_princ == 0) ? false : true; SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; kr->upn = talloc_strndup(pd, (char *)(buf + p), len); if (kr->upn == NULL) return ENOMEM; p += len; @@ -1825,13 +1825,13 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, pd->cmd == SSS_CMD_RENEW || pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM || pd->cmd == SSS_PAM_CHAUTHTOK) { SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; kr->ccname = talloc_strndup(pd, (char *)(buf + p), len); if (kr->ccname == NULL) return ENOMEM; p += len; SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; if (len > 0) { kr->old_ccname = talloc_strndup(pd, (char *)(buf + p), len); @@ -1842,7 +1842,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, } SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; kr->keytab = talloc_strndup(pd, (char *)(buf + p), len); if (kr->keytab == NULL) return ENOMEM; p += len; @@ -1875,7 +1875,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, if (pd->cmd == SSS_PAM_ACCT_MGMT) { SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; pd->user = talloc_strndup(pd, (char *)(buf + p), len); if (pd->user == NULL) return ENOMEM; p += len; diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c index 4e453b02d..fa1055eb7 100644 --- a/src/providers/krb5/krb5_child_handler.c +++ b/src/providers/krb5/krb5_child_handler.c @@ -532,9 +532,9 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len, DEBUG(SSSDBG_TRACE_LIBS, "child response [%d][%d][%d].\n", msg_status, msg_type, msg_len); - if ((p + msg_len) > len) { - DEBUG(SSSDBG_CRIT_FAILURE, "message format error [%zu] > [%zd].\n", - p+msg_len, len); + if (msg_len > len - p) { + DEBUG(SSSDBG_CRIT_FAILURE, "message format error [%d] > [%zu].\n", + msg_len, len - p); return EINVAL; } diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index 191d5bc65..7ce8d4e6c 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -66,7 +66,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, DEBUG(SSSDBG_TRACE_LIBS, "realm_str size: %d\n", len); if (len) { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->realm_str = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(SSSDBG_TRACE_LIBS, "got realm_str: %s\n", ibuf->realm_str); if (ibuf->realm_str == NULL) return ENOMEM; @@ -78,7 +78,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, DEBUG(SSSDBG_TRACE_LIBS, "princ_str size: %d\n", len); if (len) { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->princ_str = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(SSSDBG_TRACE_LIBS, "got princ_str: %s\n", ibuf->princ_str); if (ibuf->princ_str == NULL) return ENOMEM; @@ -90,7 +90,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, DEBUG(SSSDBG_TRACE_LIBS, "keytab_name size: %d\n", len); if (len) { - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ibuf->keytab_name = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(SSSDBG_TRACE_LIBS, "got keytab_name: %s\n", ibuf->keytab_name); if (ibuf->keytab_name == NULL) return ENOMEM; diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c index afe6351e9..90330f13f 100644 --- a/src/providers/ldap/sdap_child_helpers.c +++ b/src/providers/ldap/sdap_child_helpers.c @@ -222,7 +222,7 @@ static int parse_child_response(TALLOC_CTX *mem_ctx, /* ccache name size */ SAFEALIGN_COPY_UINT32_CHECK(&len, buf + p, size, &p); - if ((p + len ) > size) return EINVAL; + if (len > size - p) return EINVAL; ccn = talloc_size(mem_ctx, sizeof(char) * (len + 1)); if (ccn == NULL) { |