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 <lsleb...@redhat.com> > >>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) { bye, Sumit > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org