On (12/04/16 12:07), Sumit Bose wrote: >On Tue, Apr 12, 2016 at 11:08:54AM +0200, Pavel Březina wrote: >> On 04/11/2016 05:58 PM, Lukas Slebodnik wrote: >> >On (11/04/16 15:01), Lukas Slebodnik wrote: >> >>ehlo, >> >> >> >>attached patch fix crash in #2980 >> >> >> >>LS >> > >> >>From 422abe6e6263c3c907611a8611fa3f28d6e93ae0 Mon Sep 17 00:00:00 2001 >> >>From: Lukas Slebodnik <[email protected]> >> >>Date: Mon, 11 Apr 2016 14:46:47 +0200 >> >>Subject: [PATCH] IPA: Check RDN in ipa_add_ad_memberships_get_next >> >> >> >>LDB functions ldb_dn_get_component_val and ldb_dn_get_rdn_val >> >>validate dn before returning component value. >> >>It should be valid DN according to RFC4514. >> >> >> >>IPA/389ds might return problematic DN due to replication conflicts. >> >>e.g. "cn=System: Read Service >> >>Delegations+nsuniqueid=b0736336-d06e11e5-8e8acabe-ce8d458d,cn=permissions,dc=example,dc=com" >> >> >> >>It's better to check return value of these LDb function rather than >> >>crash because of dereference of NULL pointer. >> >> >> >>Resolves: >> >>https://fedorahosted.org/sssd/ticket/2980 >> >>--- >> >>src/providers/ipa/ipa_subdomains_ext_groups.c | 8 +++++++- >> >>1 file changed, 7 insertions(+), 1 deletion(-) >> >> >> >>diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c >> >>b/src/providers/ipa/ipa_subdomains_ext_groups.c >> >>index >> >>8e006663a31ff60b86cf6392c15ce711c52cf0fc..445538be8798d58aee5d0cabf53ce91d94467a26 >> >> 100644 >> >>--- a/src/providers/ipa/ipa_subdomains_ext_groups.c >> >>+++ b/src/providers/ipa/ipa_subdomains_ext_groups.c >> >>@@ -862,7 +862,13 @@ static void ipa_add_ad_memberships_get_next(struct >> >>tevent_req *req) >> >> goto fail; >> >> } >> >> >> >>- val = ldb_dn_get_component_val(group_dn, 0); >> >>+ val = ldb_dn_get_rdn_val(group_dn); >> >>+ if (val == NULL) { >> >>+ DEBUG(SSSDBG_OP_FAILURE, >> >>+ "Invalid group DN [%s].\n", state->groups[state->iter]); >> >>+ ret = EINVAL; >> >>+ goto fail; >> >>+ } >> > >> >Alternative solution is to validate group_dn with ldb_dn_validate >> >but it's already done in ldb_dn_get_component_val/ldb_dn_get_rdn_val >> > >> >> Did you consider using ipa_get_rdn instead? It would also check that dn has >> proper format. > >I don't think this DN check is needed here, nevertheless like in >_ipa_get_rdn() the result of ldb_dn_get_rdn_val() should be checked for > >+ if (val == NULL || val->data == NULL) { > Not a bad idea.
LS
>From a7d6e4a6bccd859f36c16a8885fa78815b8a344e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Mon, 11 Apr 2016 14:46:47 +0200 Subject: [PATCH] IPA: Check RDN in ipa_add_ad_memberships_get_next LDB functions ldb_dn_get_component_val and ldb_dn_get_rdn_val validate dn before returning component value. It should be valid DN according to RFC4514. IPA/389ds might return problematic DN due to replication conflicts. e.g. "cn=System: Read Service Delegations+nsuniqueid=b0736336-d06e11e5-8e8acabe-ce8d458d,cn=permissions,dc=example,dc=com" It's better to check return value of these LDb function rather than crash because of dereference of NULL pointer. Resolves: https://fedorahosted.org/sssd/ticket/2980 --- src/providers/ipa/ipa_subdomains_ext_groups.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c b/src/providers/ipa/ipa_subdomains_ext_groups.c index 8e006663a31ff60b86cf6392c15ce711c52cf0fc..a8ba4dfe87c81a94a1c2c51751be6745e84296d8 100644 --- a/src/providers/ipa/ipa_subdomains_ext_groups.c +++ b/src/providers/ipa/ipa_subdomains_ext_groups.c @@ -862,7 +862,13 @@ static void ipa_add_ad_memberships_get_next(struct tevent_req *req) goto fail; } - val = ldb_dn_get_component_val(group_dn, 0); + val = ldb_dn_get_rdn_val(group_dn); + if (val == NULL || val->data == NULL) { + DEBUG(SSSDBG_OP_FAILURE, + "Invalid group DN [%s].\n", state->groups[state->iter]); + ret = EINVAL; + goto fail; + } /* TODO: here is would be useful for have a filter type like BE_FILTER_DN to * directly fetch the group with the corresponding DN. */ -- 2.7.3
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
