[Freeipa-devel] [PATCH 0091] Harden dn_to_text() conversion

[Adam Tkac]

On Wed, Nov 14, 2012 at 06:02:43PM +0100, Petr Spacek wrote:
> Hello,
> 
>     Harden dn_to_text() conversion.
> 
>     All cases covered by new checks will silently cause errors
>     in other parts of plugin. This code should catch problematic
>     inputs and stop processing at the beginning.

Ack

> From 71feac726d814469291eeb413fa8c90aca02cd2d Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 14 Nov 2012 17:57:13 +0100
> Subject: [PATCH] Harden dn_to_text() conversion.
> 
> All cases covered by new checks will silently cause errors
> in other parts of plugin. This code should catch problematic
> inputs and stop processing at the beginning.
> 
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
>  src/ldap_convert.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ldap_convert.c b/src/ldap_convert.c
> index 394d8d036c1d4e0f8a82499282d1acef7d442bcd..360c7f2df976bc992213a01d652ce264202ce89e 100644
> --- a/src/ldap_convert.c
> +++ b/src/ldap_convert.c
> @@ -126,6 +126,8 @@ cleanup:
>  /**
>   * Convert LDAP DN to absolute DNS name.
>   *
> + * @param[in]  dn     LDAP DN with one or two idnsName components at the
> + *                    beginning.
>   * @param[out] target Absolute DNS name derived from the all idnsNames.
>   * @param[out] origin Absolute DNS name derived from the last idnsName
>   *                    component of DN, i.e. zone. Can be NULL.
> @@ -169,17 +171,40 @@ dn_to_text(const char *dn, ld_string_t *target, ld_string_t *origin)
>  		}
>  
>  		CHECK(explode_rdn(exploded_dn[i], &exploded_rdn, 1));
> +		if (exploded_rdn[0] == NULL || exploded_rdn[1] != NULL) {
> +			log_error("idnsName component of DN has to have "
> +				  "exactly one value: DN '%s'", dn);
> +			CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
> +		}
>  		CHECK(str_cat_char(target, exploded_rdn[0]));
>  		if (str_buf(target)[str_len(target)-1] != '.')
>  			CHECK(str_cat_char(target, "."));
>  	}
>  
> +	/* filter out unsupported cases */
> +	if (i <= 0) {
> +		log_error("no idnsName component found in DN '%s'", dn);
> +		CLEANUP_WITH(ISC_R_UNEXPECTEDEND);
> +	} else if (i == 1) { /* zone only - nothing to check */
> +		;
> +	} else if (i == 2) {
> +		if (exploded_dn[0][strlen(exploded_dn[0])-1] == '.') {
> +			log_error("absolute record name in DN "
> +				  "is not supported: DN '%s'", dn);
> +			CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
> +		}
> +	} else {
> +		log_error("unsupported number of idnsName components in DN "
> +			  "'%s': %u components found", dn, i);
> +		CLEANUP_WITH(ISC_R_NOTIMPLEMENTED);
> +	}
> +
>  	if (origin != NULL) {
>  		str_clear(origin);
>  
>  		/*
>  		 * If we have DNs with only one idnsName part,
> -		 * treat them as absolute zone name.
> +		 * treat them as absolute zone name, i.e. origin is root.
>  		 */
>  		if (i < 2)
>  			CHECK(str_init_char(origin, "."));
> -- 
> 1.7.11.7
> 


-- 
Adam Tkac, Red Hat, Inc.