[SSSD] [PATCH] Sanitize ldap attributes in the config file
Sumit Bose
sbose at redhat.com
Mon Nov 15 12:34:14 UTC 2010
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 at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list