Re: (ITS#7995) of-by-one error in schema

[hyc@symas.com]

Ð?еонид ЮÑ?Ñ?ев wrote:
> (2)
> But let see to lines 570-575 of mdb-back/attr.c and lines 764-772 of slapd/ad.c:
> - does the mdb-value include a NUL byte =
> https://github.com/leo-yuriev/openldap-lmdb-challenge/blob/2.4-devel/servers/slapd/back-mdb/attr.c#L570?
> - if "YES" then "+1" at slapd/ad.c:764 is wrong =
> https://github.com/leo-yuriev/openldap-lmdb-challenge/blob/2.4-devel/servers/slapd/ad.c#L764
> - but if "NO" then "strcpy()" at slapd/ad.c:772 (and more) is wrong =
> https://github.com/leo-yuriev/openldap-lmdb-challenge/blob/2.4-devel/servers/slapd/ad.c#L772

And the answer is no, back-mdb/attr.c mdb_ad_get() doesn't store the 
trailing NUL byte,

> diff --git a/servers/slapd/ad.c b/servers/slapd/ad.c
> index 246b900..fd60483 100644
> --- a/servers/slapd/ad.c
> +++ b/servers/slapd/ad.c
> @@ -118,7 +118,7 @@ AttributeDescription * ad_find_tags(
>  	for (ad = type->sat_ad; ad; ad=ad->ad_next)
>  	{
>  		if (ad->ad_tags.bv_len == tags->bv_len &&
> -			!strcasecmp(ad->ad_tags.bv_val, tags->bv_val))
> +			!strncasecmp( ad->ad_tags.bv_val, tags->bv_val, ad->ad_tags.bv_len ))
>  			break;
>  	}

Unnecessary.

>  	ldap_pvt_thread_mutex_unlock( &type->sat_ad_mutex );
> @@ -742,14 +742,13 @@ int slap_bv2undef_ad(
>  	/* use the appropriate type */
>  	if ( flags & SLAP_AD_PROXIED ) {
>  		at = slap_schema.si_at_proxied;
> -
>  	} else {
>  		at = slap_schema.si_at_undefined;
>  	}
>
>  	for( desc = at->sat_ad; desc; desc=desc->ad_next ) {
>  		if( desc->ad_cname.bv_len == bv->bv_len &&
> -		    !strcasecmp( desc->ad_cname.bv_val, bv->bv_val ) )
> +		    !strncasecmp( desc->ad_cname.bv_val, bv->bv_val, desc->ad_cname.bv_len) )

Unnecessary. We've already checked that the two lengths are equal; even 
if bv->bv_val is non-terminated the compare will stop because 
desc->ad_cname is terminated.

>  		{
>  		    	break;
>  		}
> @@ -769,7 +768,8 @@ int slap_bv2undef_ad(
>
>  		desc->ad_cname.bv_len = bv->bv_len;
>  		desc->ad_cname.bv_val = (char *)(desc+1);
> -		strcpy(desc->ad_cname.bv_val, bv->bv_val);
> +		strncpy( desc->ad_cname.bv_val, bv->bv_val, desc->ad_cname.bv_len )
> +			[desc->ad_cname.bv_len] = '\0';

This is a valid fix.
>
>  		/* canonical to upper case */
>  		ldap_pvt_str2upper( desc->ad_cname.bv_val );
> @@ -806,9 +806,10 @@ slap_bv2tmp_ad(
>  		 slap_sl_mfuncs.bmf_malloc( sizeof(AttributeDescription) +
>  			bv->bv_len + 1, memctx );
>
> -	ad->ad_cname.bv_val = (char *)(ad+1);
> -	strncpy( ad->ad_cname.bv_val, bv->bv_val, bv->bv_len+1 );
>  	ad->ad_cname.bv_len = bv->bv_len;
> +	ad->ad_cname.bv_val = (char *)(ad+1);
> +	strncpy( ad->ad_cname.bv_val, bv->bv_val, ad->ad_cname.bv_len)
> +		[ad->ad_cname.bv_len] = '\0';
>  	ad->ad_flags = SLAP_DESC_TEMPORARY;
>  	ad->ad_type = slap_schema.si_at_undefined;
>
Unnecessary.

> @@ -887,7 +888,7 @@ an_find(
>
>  	for ( ; a->an_name.bv_val; a++ ) {
>  		if ( a->an_name.bv_len != s->bv_len) continue;
> -		if ( strcasecmp( s->bv_val, a->an_name.bv_val ) == 0 ) {
> +		if ( strncasecmp( s->bv_val, a->an_name.bv_val, s->bv_len ) == 0 ) {
>  			return( 1 );
>  		}
>  	}

Unnecessary.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/