On Mon, Nov 15, 2010 at 07:06:31AM -0500, Stephen Gallagher wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 11/15/2010 07:01 AM, Sumit Bose wrote: > > On Fri, Nov 12, 2010 at 10:12:51AM -0500, Stephen Gallagher wrote: > >> -----BEGIN PGP SIGNED MESSAGE----- > >> Hash: SHA1 > >> > >> https://fedorahosted.org/sssd/ticket/458 > >> > >> Previously, it was possible to perform a sort of LDAP filter injection > >> with careful crafting of the ldap attributes in the config file. > >> > >> This guarantees that any attribute specified in the config file is > >> escaped properly, resulting in an inability to inject subfilters. > >> > >> > >> Note: this was not a security issue, as it was editable only by root and > >> even then, all checks were performed on the server, not the client. > >> > >> + > >> + if (name) { > >> + ret = sss_filter_sanitize(map, name, &map[i].name); > >> + talloc_zfree(name); > > > > NACK, please check ret. > > > > bye, > > Sumit > > > >> + } else { > >> + map[i].name = NULL; > >> + } > >> + > > > ret is checked just after that else statement. > > if ((ret != EOK) || (map[i].def_name && !map[i].name)) { >
ah, sorry, I should have read the context. But after reading it I still have comments: @@ -50,7 +51,22 @@ int sdap_get_map(TALLOC_CTX *memctx, ret = confdb_get_string(cdb, map, conf_path, map[i].opt_name, map[i].def_name, - &map[i].name); + &name); + if (ret != EOK) { + DEBUG(0, ("Failed to retrieve value for %s\n", map[i].opt_name)); + if (ret != EOK) { this 'if' is redundant + talloc_zfree(map); + return EINVAL; + } + } + + if (name) { + ret = sss_filter_sanitize(map, name, &map[i].name); + talloc_zfree(name); + } else { + map[i].name = NULL; + } + if ((ret != EOK) || (map[i].def_name && !map[i].name)) { DEBUG(0, ("Failed to retrieve value for %s\n", map[i].opt_name)); If I remember correctly there is no debugging output in sss_filter_sanitize(), so I think it would be helpful to have a more specific message if either the retrieval or the sanitation failed. bye, Sumit if (ret != EOK) { > > > > - -- > Stephen Gallagher > RHCE 804006346421761 > > Delivering value year after year. > Red Hat ranks #1 in value among software vendors. > http://www.redhat.com/promo/vendor/ > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.11 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iEYEARECAAYFAkzhIkIACgkQeiVVYja6o6N1YwCdHrFW/4qDWGsmfzlDtw7hBXw8 > GZUAnjao6FzI4R8G2xuM31UbB+GnVgVJ > =MPNC > -----END PGP SIGNATURE----- > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel