From: Pierangelo Masarati Date: Sat, 29 Sep 2007 17:55:33 +0000 (+0000) Subject: first round of s/sprintf/snprintf/; the rationale is: truncate error messages rather... X-Git-Tag: OPENLDAP_REL_ENG_2_4_9~20^2~581 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=2de9d97ad2cee5aa517e9ad025efe7f82f5098c1;p=openldap first round of s/sprintf/snprintf/; the rationale is: truncate error messages rather than buffer overflow; otherwise, assert that no buffer overflow occurred. In some cases, error handling could be added. Please note: no real error in the code was found so far, apart from that in syncprov.c and from some config-time error logging; however, errors could slip thru again if things don't change consistently --- diff --git a/servers/slapd/overlays/accesslog.c b/servers/slapd/overlays/accesslog.c index e0b0e8bb02..ff04ea5dd0 100644 --- a/servers/slapd/overlays/accesslog.c +++ b/servers/slapd/overlays/accesslog.c @@ -515,11 +515,13 @@ log_age_parse(char *agestr) } static void -log_age_unparse( int age, struct berval *agebv ) +log_age_unparse( int age, struct berval *agebv, size_t size ) { - int dd, hh, mm, ss; + int dd, hh, mm, ss, len; char *ptr; + assert( size > 0 ); + ss = age % 60; age /= 60; mm = age % 60; @@ -530,11 +532,22 @@ log_age_unparse( int age, struct berval *agebv ) ptr = agebv->bv_val; - if ( dd ) - ptr += sprintf( ptr, "%d+", dd ); - ptr += sprintf( ptr, "%02d:%02d", hh, mm ); - if ( ss ) - ptr += sprintf( ptr, ":%02d", ss ); + if ( dd ) { + len = snprintf( ptr, size, "%d+", dd ); + assert( len >= 0 && len < size ); + size -= len; + ptr += len; + } + len = snprintf( ptr, size, "%02d:%02d", hh, mm ); + assert( len >= 0 && len < size ); + size -= len; + ptr += len; + if ( ss ) { + len = snprintf( ptr, size, ":%02d", ss ); + assert( len >= 0 && len < size ); + size -= len; + ptr += len; + } agebv->bv_len = ptr - agebv->bv_val; } @@ -704,11 +717,11 @@ log_cf_gen(ConfigArgs *c) break; } agebv.bv_val = agebuf; - log_age_unparse( li->li_age, &agebv ); + log_age_unparse( li->li_age, &agebv, sizeof( agebuf ) ); agebv.bv_val[agebv.bv_len] = ' '; agebv.bv_len++; cyclebv.bv_val = agebv.bv_val + agebv.bv_len; - log_age_unparse( li->li_cycle, &cyclebv ); + log_age_unparse( li->li_cycle, &cyclebv, sizeof( agebuf ) - agebv.bv_len ); agebv.bv_len += cyclebv.bv_len; value_add_one( &c->rvalue_vals, &agebv ); break; @@ -845,7 +858,7 @@ log_cf_gen(ConfigArgs *c) case LOG_OLD: li->li_oldf = str2filter( c->argv[1] ); if ( !li->li_oldf ) { - sprintf( c->cr_msg, "bad filter!" ); + snprintf( c->cr_msg, sizeof( c->cr_msg ), "bad filter!" ); rc = 1; } break; @@ -1165,8 +1178,8 @@ static Entry *accesslog_entry( Operation *op, SlapReply *rs, int logop, timestamp.bv_val = rdnbuf+STRLENOF(RDNEQ); timestamp.bv_len = sizeof(rdnbuf) - STRLENOF(RDNEQ); slap_timestamp( &op->o_time, ×tamp ); - sprintf( timestamp.bv_val + timestamp.bv_len-1, ".%06dZ", op->o_tincr ); - timestamp.bv_len += 7; + snprintf( timestamp.bv_val + timestamp.bv_len-1, sizeof(".123456Z"), ".%06dZ", op->o_tincr ); + timestamp.bv_len += STRLENOF(".123456"); rdn.bv_len = STRLENOF(RDNEQ)+timestamp.bv_len; ad_reqStart->ad_type->sat_equality->smr_normalize( @@ -1190,8 +1203,8 @@ static Entry *accesslog_entry( Operation *op, SlapReply *rs, int logop, timestamp.bv_len = sizeof(rdnbuf) - STRLENOF(RDNEQ); slap_timestamp( &op2->o_time, ×tamp ); - sprintf( timestamp.bv_val + timestamp.bv_len-1, ".%06dZ", op2->o_tincr ); - timestamp.bv_len += 7; + snprintf( timestamp.bv_val + timestamp.bv_len-1, sizeof(".123456Z"), ".%06dZ", op2->o_tincr ); + timestamp.bv_len += STRLENOF(".123456"); attr_merge_normalize_one( e, ad_reqEnd, ×tamp, op->o_tmpmemctx ); @@ -1210,8 +1223,10 @@ static Entry *accesslog_entry( Operation *op, SlapReply *rs, int logop, attr_merge_one( e, ad_reqType, &lo->word, NULL ); } - rdn.bv_len = sprintf( rdn.bv_val, "%lu", op->o_connid ); - attr_merge_one( e, ad_reqSession, &rdn, NULL ); + rdn.bv_len = snprintf( rdn.bv_val, sizeof( rdnbuf ), "%lu", op->o_connid ); + if ( rdn.bv_len >= 0 || rdn.bv_len < sizeof( rdnbuf ) ) { + attr_merge_one( e, ad_reqSession, &rdn, NULL ); + } /* else? */ if ( BER_BVISNULL( &op->o_dn ) ) { attr_merge_one( e, ad_reqAuthzID, (struct berval *)&slap_empty_bv, @@ -1340,10 +1355,11 @@ static int accesslog_response(Operation *op, SlapReply *rs) { ber_str2bv( rs->sr_text, 0, 0, &bv ); attr_merge_one( e, ad_reqMessage, &bv, NULL ); } - bv.bv_len = sprintf( timebuf, "%d", rs->sr_err ); - bv.bv_val = timebuf; - - attr_merge_one( e, ad_reqResult, &bv, NULL ); + bv.bv_len = snprintf( timebuf, sizeof( timebuf ), "%d", rs->sr_err ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( timebuf ) ) { + bv.bv_val = timebuf; + attr_merge_one( e, ad_reqResult, &bv, NULL ); + } last_attr = attr_find( e->e_attrs, ad_reqResult ); @@ -1556,20 +1572,28 @@ static int accesslog_response(Operation *op, SlapReply *rs) { op->o_tmpfree( vals, op->o_tmpmemctx ); } bv.bv_val = timebuf; - bv.bv_len = sprintf( bv.bv_val, "%d", rs->sr_nentries ); - attr_merge_one( e, ad_reqEntries, &bv, NULL ); - - bv.bv_len = sprintf( bv.bv_val, "%d", op->ors_tlimit ); - attr_merge_one( e, ad_reqTimeLimit, &bv, NULL ); - - bv.bv_len = sprintf( bv.bv_val, "%d", op->ors_slimit ); - attr_merge_one( e, ad_reqSizeLimit, &bv, NULL ); + bv.bv_len = snprintf( bv.bv_val, sizeof( timebuf ), "%d", rs->sr_nentries ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( timebuf ) ) { + attr_merge_one( e, ad_reqEntries, &bv, NULL ); + } /* else? */ + + bv.bv_len = snprintf( bv.bv_val, sizeof( timebuf ), "%d", op->ors_tlimit ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( timebuf ) ) { + attr_merge_one( e, ad_reqTimeLimit, &bv, NULL ); + } /* else? */ + + bv.bv_len = snprintf( bv.bv_val, sizeof( timebuf ), "%d", op->ors_slimit ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( timebuf ) ) { + attr_merge_one( e, ad_reqSizeLimit, &bv, NULL ); + } /* else? */ break; case LOG_EN_BIND: bv.bv_val = timebuf; - bv.bv_len = sprintf( bv.bv_val, "%d", op->o_protocol ); - attr_merge_one( e, ad_reqVersion, &bv, NULL ); + bv.bv_len = snprintf( bv.bv_val, sizeof( timebuf ), "%d", op->o_protocol ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( timebuf ) ) { + attr_merge_one( e, ad_reqVersion, &bv, NULL ); + } /* else? */ if ( op->orb_method == LDAP_AUTH_SIMPLE ) { attr_merge_one( e, ad_reqMethod, &simple, NULL ); } else { @@ -1743,8 +1767,10 @@ accesslog_abandon( Operation *op, SlapReply *rs ) e = accesslog_entry( op, rs, LOG_EN_ABANDON, &op2 ); bv.bv_val = buf; - bv.bv_len = sprintf( buf, "%d", op->orn_msgid ); - attr_merge_one( e, ad_reqId, &bv, NULL ); + bv.bv_len = snprintf( buf, sizeof( buf ), "%d", op->orn_msgid ); + if ( bv.bv_len >= 0 && bv.bv_len < sizeof( buf ) ) { + attr_merge_one( e, ad_reqId, &bv, NULL ); + } /* else? */ op2.o_hdr = op->o_hdr; op2.o_tag = LDAP_REQ_ADD; diff --git a/servers/slapd/overlays/collect.c b/servers/slapd/overlays/collect.c index d78f32b54a..8dae65a078 100644 --- a/servers/slapd/overlays/collect.c +++ b/servers/slapd/overlays/collect.c @@ -58,12 +58,14 @@ collect_cf( ConfigArgs *c ) collect_info *ci; for ( ci = on->on_bi.bi_private; ci; ci = ci->ci_next ) { struct berval bv; + int len; - bv.bv_len = ci->ci_dn.bv_len + 3 + + bv.bv_len = ci->ci_dn.bv_len + STRLENOF("\"\" ") + ci->ci_ad->ad_cname.bv_len; bv.bv_val = ch_malloc( bv.bv_len + 1 ); - sprintf( bv.bv_val, "\"%s\" %s", ci->ci_dn.bv_val, - ci->ci_ad->ad_cname.bv_val ); + len = snprintf( bv.bv_val, bv.bv_len + 1, "\"%s\" %s", + ci->ci_dn.bv_val, ci->ci_ad->ad_cname.bv_val ); + assert( len == bv.bv_len ); ber_bvarray_add( &c->rvalue_vals, &bv ); rc = 0; } diff --git a/servers/slapd/overlays/dynlist.c b/servers/slapd/overlays/dynlist.c index ef75f01167..687051b016 100644 --- a/servers/slapd/overlays/dynlist.c +++ b/servers/slapd/overlays/dynlist.c @@ -1360,7 +1360,8 @@ dynlist_db_open( if ( oc == NULL ) { oc = oc_find( "groupOfURLs" ); if ( oc == NULL ) { - sprintf( cr->msg, "unable to fetch objectClass \"groupOfURLs\"" ); + snprintf( cr->msg, sizeof( cr->msg), + "unable to fetch objectClass \"groupOfURLs\"" ); Debug( LDAP_DEBUG_ANY, "dynlist_db_open: %s.\n", cr->msg, 0, 0 ); return 1; } @@ -1373,7 +1374,8 @@ dynlist_db_open( if ( ad == NULL ) { rc = slap_str2ad( "memberURL", &ad, &text ); if ( rc != LDAP_SUCCESS ) { - sprintf( cr->msg, "unable to fetch attributeDescription \"memberURL\": %d (%s)", + snprintf( cr->msg, sizeof( cr->msg), + "unable to fetch attributeDescription \"memberURL\": %d (%s)", rc, text ); Debug( LDAP_DEBUG_ANY, "dynlist_db_open: %s.\n", cr->msg, 0, 0 ); return 1; diff --git a/servers/slapd/overlays/pcache.c b/servers/slapd/overlays/pcache.c index 70ef66ac55..6a20a1efc1 100644 --- a/servers/slapd/overlays/pcache.c +++ b/servers/slapd/overlays/pcache.c @@ -1482,67 +1482,91 @@ filter2template( int* filter_got_oc ) { AttributeDescription *ad; + int len, ret; switch ( f->f_choice ) { case LDAP_FILTER_EQUALITY: ad = f->f_av_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s=)", ad->ad_cname.bv_val ); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(=)") - 1 ); + len = STRLENOF( "(=)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s=)", ad->ad_cname.bv_val ); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_GE: ad = f->f_av_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s>=)", ad->ad_cname.bv_val); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(>=)") - 1 ); + len = STRLENOF( "(>=)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s>=)", ad->ad_cname.bv_val); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_LE: ad = f->f_av_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s<=)", ad->ad_cname.bv_val); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(<=)") - 1 ); + len = STRLENOF( "(<=)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s<=)", ad->ad_cname.bv_val); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_APPROX: ad = f->f_av_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s~=)", ad->ad_cname.bv_val); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(~=)") - 1 ); + len = STRLENOF( "(~=)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s~=)", ad->ad_cname.bv_val); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_SUBSTRINGS: ad = f->f_sub_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s=)", ad->ad_cname.bv_val ); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(=)") - 1 ); + len = STRLENOF( "(=)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s=)", ad->ad_cname.bv_val ); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_PRESENT: ad = f->f_desc; - sprintf( fstr->bv_val+fstr->bv_len, "(%s=*)", ad->ad_cname.bv_val ); - fstr->bv_len += ad->ad_cname.bv_len + ( sizeof("(=*)") - 1 ); + len = STRLENOF( "(=*)" ) + ad->ad_cname.bv_len; + ret = snprintf( fstr->bv_val+fstr->bv_len, len + 1, "(%s=*)", ad->ad_cname.bv_val ); + assert( ret == len ); + fstr->bv_len += len; break; case LDAP_FILTER_AND: case LDAP_FILTER_OR: case LDAP_FILTER_NOT: { int rc = 0; - sprintf( fstr->bv_val+fstr->bv_len, "(%c", - f->f_choice == LDAP_FILTER_AND ? '&' : - f->f_choice == LDAP_FILTER_OR ? '|' : '!' ); - fstr->bv_len += sizeof("(%") - 1; + fstr->bv_val[fstr->bv_len++] = '('; + switch ( f->f_choice ) { + case LDAP_FILTER_AND: + fstr->bv_val[fstr->bv_len] = '&'; + break; + case LDAP_FILTER_OR: + fstr->bv_val[fstr->bv_len] = '|'; + break; + case LDAP_FILTER_NOT: + fstr->bv_val[fstr->bv_len] = '!'; + break; + } + fstr->bv_len++; for ( f = f->f_list; f != NULL; f = f->f_next ) { rc = filter2template( op, f, fstr, filter_attrs, filter_cnt, filter_got_oc ); if ( rc ) break; } - sprintf( fstr->bv_val+fstr->bv_len, ")" ); - fstr->bv_len += sizeof(")") - 1; + fstr->bv_val[fstr->bv_len++] = ')'; + fstr->bv_val[fstr->bv_len] = '\0'; return rc; } default: - strcpy( fstr->bv_val, "(?=?)" ); - fstr->bv_len += sizeof("(?=?)")-1; + /* a filter should at least have room for "()", + * an "=" and for a 1-char attr */ + strcpy( fstr->bv_val, "(?=)" ); + fstr->bv_len += STRLENOF("(?=)"); return -1; } @@ -2626,7 +2650,11 @@ pc_cfadd( Operation *op, SlapReply *rs, Entry *p, ConfigArgs *ca ) struct berval bv; /* FIXME: should not hardcode "olcDatabase" here */ - bv.bv_len = sprintf( ca->cr_msg, "olcDatabase=%s", cm->db.bd_info->bi_type ); + bv.bv_len = snprintf( ca->cr_msg, sizeof( ca->cr_msg ), + "olcDatabase=%s", cm->db.bd_info->bi_type ); + if ( bv.bv_len < 0 || bv.bv_len >= sizeof( ca->cr_msg ) ) { + return -1; + } bv.bv_val = ca->cr_msg; ca->be = &cm->db; diff --git a/servers/slapd/overlays/syncprov.c b/servers/slapd/overlays/syncprov.c index 9cae2c53dc..942d387f2c 100644 --- a/servers/slapd/overlays/syncprov.c +++ b/servers/slapd/overlays/syncprov.c @@ -624,8 +624,11 @@ again: maxid = i; } } - fop.ors_filterstr.bv_len = sprintf( buf, "(entryCSN>=%s)", - cf.f_av_value.bv_val ); + fop.ors_filterstr.bv_len = snprintf( buf, sizeof( buf ), + "(entryCSN>=%s)", cf.f_av_value.bv_val ); + if ( fop.ors_filterstr.bv_len < 0 || fop.ors_filterstr.bv_len >= sizeof( buf ) ) { + return LDAP_OTHER; + } fop.ors_attrsonly = 0; fop.ors_attrs = csn_anlist; fop.ors_slimit = SLAP_NO_LIMIT; @@ -649,16 +652,19 @@ again: /* Look for exact match the first time */ if ( findcsn_retry ) { cf.f_choice = LDAP_FILTER_EQUALITY; - fop.ors_filterstr.bv_len = sprintf( buf, "(entryCSN=%s)", - cf.f_av_value.bv_val ); + fop.ors_filterstr.bv_len = snprintf( buf, sizeof( buf ), + "(entryCSN=%s)", cf.f_av_value.bv_val ); /* On retry, look for <= */ } else { cf.f_choice = LDAP_FILTER_LE; fop.ors_limit = &fc_limits; memset( &fc_limits, 0, sizeof( fc_limits )); fc_limits.lms_s_unchecked = 1; - fop.ors_filterstr.bv_len = sprintf( buf, "(entryCSN<=%s)", - cf.f_av_value.bv_val ); + fop.ors_filterstr.bv_len = snprintf( buf, sizeof( buf ), + "(entryCSN<=%s)", cf.f_av_value.bv_val ); + } + if ( fop.ors_filterstr.bv_len < 0 || fop.ors_filterstr.bv_len >= sizeof( buf ) ) { + return LDAP_OTHER; } fop.ors_attrsonly = 1; fop.ors_attrs = slap_anlist_no_attrs; @@ -2368,10 +2374,14 @@ sp_cf_gen(ConfigArgs *c) case SP_CHKPT: if ( si->si_chkops || si->si_chktime ) { struct berval bv; - bv.bv_len = sprintf( c->cr_msg, "%d %d", - si->si_chkops, si->si_chktime ); - bv.bv_val = c->cr_msg; - value_add_one( &c->rvalue_vals, &bv ); + bv.bv_len = snprintf( c->cr_msg, sizeof( c->cr_msg ), + "%d %d", si->si_chkops, si->si_chktime ); + if ( bv.bv_len < 0 || bv.bv_len >= sizeof( c->cr_msg ) ) { + rc = 1; + } else { + bv.bv_val = c->cr_msg; + value_add_one( &c->rvalue_vals, &bv ); + } } else { rc = 1; } diff --git a/servers/slapd/overlays/translucent.c b/servers/slapd/overlays/translucent.c index db9079692f..40614d8252 100644 --- a/servers/slapd/overlays/translucent.c +++ b/servers/slapd/overlays/translucent.c @@ -103,8 +103,11 @@ translucent_cfadd( Operation *op, SlapReply *rs, Entry *e, ConfigArgs *ca ) Debug(LDAP_DEBUG_TRACE, "==> translucent_cfadd\n", 0, 0, 0); /* FIXME: should not hardcode "olcDatabase" here */ - bv.bv_len = sprintf( ca->cr_msg, "olcDatabase=%s", - ov->db.bd_info->bi_type ); + bv.bv_len = snprintf( ca->cr_msg, sizeof( ca->cr_msg ), + "olcDatabase=%s", ov->db.bd_info->bi_type ); + if ( bv.bv_len < 0 || bv.bv_len >= sizeof( ca->cr_msg ) ) { + return -1; + } bv.bv_val = ca->cr_msg; ca->be = &ov->db; diff --git a/servers/slapd/overlays/unique.c b/servers/slapd/overlays/unique.c index 68154eaec3..32eabf0f05 100644 --- a/servers/slapd/overlays/unique.c +++ b/servers/slapd/overlays/unique.c @@ -198,7 +198,7 @@ unique_new_domain_uri ( unique_domain_uri **urip, } if ( !dnIsSuffix ( uri->ndn, &be->be_nsuffix[0] ) ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "dn <%s> is not a suffix of backend base dn <%s>", uri->dn->bv_val, be->be_nsuffix[0].bv_val ); @@ -401,7 +401,7 @@ unique_cf_base( ConfigArgs *c ) case LDAP_MOD_ADD: case SLAP_CONFIG_ADD: if ( domains ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "cannot set legacy attrs when URIs are present" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -410,7 +410,7 @@ unique_cf_base( ConfigArgs *c ) } if ( !dnIsSuffix ( &c->value_ndn, &be->be_nsuffix[0] ) ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "dn is not a suffix of backend base" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -499,7 +499,7 @@ unique_cf_attrs( ConfigArgs *c ) case LDAP_MOD_ADD: case SLAP_CONFIG_ADD: if ( domains ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "cannot set legacy attrs when URIs are present" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -510,7 +510,7 @@ unique_cf_attrs( ConfigArgs *c ) && legacy->uri && legacy->uri->attrs && (c->type == UNIQUE_IGNORE) != legacy->ignore ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "cannot set both attrs and ignore-attrs" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -620,7 +620,7 @@ unique_cf_strict( ConfigArgs *c ) case LDAP_MOD_ADD: case SLAP_CONFIG_ADD: if ( domains ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "cannot set legacy attrs when URIs are present" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -705,7 +705,7 @@ unique_cf_uri( ConfigArgs *c ) case SLAP_CONFIG_ADD: /* fallthrough */ case LDAP_MOD_ADD: if ( legacy ) { - sprintf ( c->cr_msg, + snprintf( c->cr_msg, sizeof( c->cr_msg ), "cannot set Uri when legacy attrs are present" ); Debug ( LDAP_DEBUG_CONFIG, "unique config: %s\n", c->cr_msg, NULL, NULL ); @@ -902,6 +902,7 @@ build_filter( AttributeDescription *ad, BerVarray b, char *kp, + int ks, void *ctx ) { @@ -923,15 +924,21 @@ build_filter( if ( b && b[0].bv_val ) { for ( i = 0; b[i].bv_val; i++ ) { struct berval bv; + int len; ldap_bv2escaped_filter_value_x( &b[i], &bv, 1, ctx ); - kp += sprintf( kp, "(%s=%s)", ad->ad_cname.bv_val, bv.bv_val ); + len = snprintf( kp, ks, "(%s=%s)", ad->ad_cname.bv_val, bv.bv_val ); + assert( len >= 0 && len < ks ); + kp += len; if ( bv.bv_val != b[i].bv_val ) { ber_memfree_x( bv.bv_val, ctx ); } } } else if ( domain->strict ) { - kp += sprintf( kp, "(%s=*)", ad->ad_cname.bv_val ); + int len; + len = snprintf( kp, ks, "(%s=*)", ad->ad_cname.bv_val ); + assert( len >= 0 && len < ks ); + kp += len; } break; } @@ -1020,13 +1027,16 @@ unique_add( for ( domain = legacy ? legacy : domains; domain; - domain = domain->next ) { + domain = domain->next ) + { unique_domain_uri *uri; int ks = 0; for ( uri = domain->uri; uri; - uri = uri->next ) { + uri = uri->next ) + { + int len; if ( uri->ndn && !dnIsSuffix( &op->o_req_ndn, uri->ndn )) @@ -1055,9 +1065,14 @@ unique_add( ks += uri->filter->bv_len + STRLENOF ("(&)"); kp = key = op->o_tmpalloc(ks, op->o_tmpmemctx); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, "(&%s", uri->filter->bv_val); - kp += sprintf(kp, "(|"); + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf (kp, ks, "(&%s", uri->filter->bv_val); + assert( len >= 0 && len < ks ); + kp += len; + } + len = snprintf(kp, ks - (kp - key), "(|"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; for(a = op->ora_e->e_attrs; a; a = a->a_next) kp = build_filter(domain, @@ -1065,11 +1080,17 @@ unique_add( a->a_desc, a->a_vals, kp, + ks - ( kp - key ), op->o_tmpmemctx); - kp += sprintf(kp, ")"); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, ")"); + len = snprintf(kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf(kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + } rc = unique_search ( op, &nop, @@ -1110,13 +1131,16 @@ unique_modify( for ( domain = legacy ? legacy : domains; domain; - domain = domain->next ) { + domain = domain->next ) + { unique_domain_uri *uri; int ks = 0; for ( uri = domain->uri; uri; - uri = uri->next ) { + uri = uri->next ) + { + int len; if ( uri->ndn && !dnIsSuffix( &op->o_req_ndn, uri->ndn )) @@ -1146,9 +1170,14 @@ unique_modify( ks += uri->filter->bv_len + STRLENOF ("(&)"); kp = key = op->o_tmpalloc(ks, op->o_tmpmemctx); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, "(&%s", uri->filter->bv_val); - kp += sprintf(kp, "(|"); + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf(kp, ks, "(&%s", uri->filter->bv_val); + assert( len >= 0 && len < ks ); + kp += len; + } + len = snprintf(kp, ks - (kp - key), "(|"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; for(m = op->orm_modlist; m; m = m->sml_next) if ( (m->sml_op & LDAP_MOD_OP) @@ -1158,11 +1187,17 @@ unique_modify( m->sml_desc, m->sml_values, kp, + ks - (kp - key), op->o_tmpmemctx ); - kp += sprintf (kp, ")"); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, ")"); + len = snprintf(kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf (kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + } rc = unique_search ( op, &nop, @@ -1204,14 +1239,16 @@ unique_modrdn( for ( domain = legacy ? legacy : domains; domain; - domain = domain->next ) { + domain = domain->next ) + { unique_domain_uri *uri; int ks = 0; for ( uri = domain->uri; uri; - uri = uri->next ) { - int i; + uri = uri->next ) + { + int i, len; if ( uri->ndn && !dnIsSuffix( &op->o_req_ndn, uri->ndn ) @@ -1263,9 +1300,14 @@ unique_modrdn( ks += uri->filter->bv_len + STRLENOF ("(&)"); kp = key = op->o_tmpalloc(ks, op->o_tmpmemctx); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, "(&%s", uri->filter->bv_val); - kp += sprintf(kp, "(|"); + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf(kp, ks, "(&%s", uri->filter->bv_val); + assert( len >= 0 && len < ks ); + kp += len; + } + len = snprintf(kp, ks - (kp - key), "(|"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; for ( i=0; newrdn[i]; i++) { bv[0] = newrdn[i]->la_value; @@ -1274,12 +1316,18 @@ unique_modrdn( newrdn[i]->la_private, bv, kp, + ks - (kp - key ), op->o_tmpmemctx); } - kp += sprintf(kp, ")"); - if ( uri->filter && uri->filter->bv_len ) - kp += sprintf (kp, ")"); + len = snprintf(kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + if ( uri->filter && uri->filter->bv_len ) { + len = snprintf (kp, ks - (kp - key), ")"); + assert( len >= 0 && len < ks - (kp - key) ); + kp += len; + } rc = unique_search ( op, &nop, diff --git a/servers/slapd/overlays/valsort.c b/servers/slapd/overlays/valsort.c index 5cc06cfaf0..0e4a832262 100644 --- a/servers/slapd/overlays/valsort.c +++ b/servers/slapd/overlays/valsort.c @@ -152,13 +152,13 @@ valsort_cf_func(ConfigArgs *c) { vitmp.vi_ad = NULL; i = slap_str2ad( c->argv[1], &vitmp.vi_ad, &text ); if ( i ) { - sprintf( c->cr_msg, "<%s> %s", c->argv[0], text ); + snprintf( c->cr_msg, sizeof( c->cr_msg), "<%s> %s", c->argv[0], text ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[1] ); return(1); } if ( is_at_single_value( vitmp.vi_ad->ad_type )) { - sprintf( c->cr_msg, "<%s> %s is single-valued, ignoring", c->argv[0], + snprintf( c->cr_msg, sizeof( c->cr_msg ), "<%s> %s is single-valued, ignoring", c->argv[0], vitmp.vi_ad->ad_cname.bv_val ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[1] ); @@ -170,14 +170,14 @@ valsort_cf_func(ConfigArgs *c) { ber_str2bv( c->argv[2], 0, 0, &bv ); i = dnNormalize( 0, NULL, NULL, &bv, &vitmp.vi_dn, NULL ); if ( i ) { - sprintf( c->cr_msg, "<%s> unable to normalize DN", c->argv[0] ); + snprintf( c->cr_msg, sizeof( c->cr_msg ), "<%s> unable to normalize DN", c->argv[0] ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[2] ); return(1); } i = verb_to_mask( c->argv[3], sorts ); if ( BER_BVISNULL( &sorts[i].word )) { - sprintf( c->cr_msg, "<%s> unrecognized sort type", c->argv[0] ); + snprintf( c->cr_msg, sizeof( c->cr_msg ), "<%s> unrecognized sort type", c->argv[0] ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[3] ); return(1); @@ -186,7 +186,7 @@ valsort_cf_func(ConfigArgs *c) { if ( sorts[i].mask == VALSORT_WEIGHTED && c->argc == 5 ) { i = verb_to_mask( c->argv[4], sorts ); if ( BER_BVISNULL( &sorts[i].word )) { - sprintf( c->cr_msg, "<%s> unrecognized sort type", c->argv[0] ); + snprintf( c->cr_msg, sizeof( c->cr_msg ), "<%s> unrecognized sort type", c->argv[0] ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[4] ); return(1); @@ -194,7 +194,7 @@ valsort_cf_func(ConfigArgs *c) { vitmp.vi_sort |= sorts[i].mask; } if (( vitmp.vi_sort & VALSORT_NUMERIC ) && !is_numeric ) { - sprintf( c->cr_msg, "<%s> numeric sort specified for non-numeric syntax", + snprintf( c->cr_msg, sizeof( c->cr_msg ), "<%s> numeric sort specified for non-numeric syntax", c->argv[0] ); Debug( LDAP_DEBUG_ANY, "%s: %s (%s)!\n", c->log, c->cr_msg, c->argv[1] ); diff --git a/servers/slapd/shell-backends/passwd-shell.c b/servers/slapd/shell-backends/passwd-shell.c index b32de501d3..15d0b81479 100644 --- a/servers/slapd/shell-backends/passwd-shell.c +++ b/servers/slapd/shell-backends/passwd-shell.c @@ -144,8 +144,6 @@ pw2entry( struct ldop *op, struct passwd *pw ) struct ldattr *attr; int i; - entry = (struct ldentry *) ecalloc( 1, sizeof( struct ldentry )); - /* * construct the DN from pw_name */ @@ -153,13 +151,19 @@ pw2entry( struct ldop *op, struct passwd *pw ) /* * X.500 style DN */ - sprintf( tmpbuf, "cn=%s, %s", pw->pw_name, op->ldop_suffixes[ 0 ] ); + i = snprintf( tmpbuf, sizeof( tmpbuf ), "cn=%s, %s", pw->pw_name, op->ldop_suffixes[ 0 ] ); } else { /* * RFC-822 style DN */ - sprintf( tmpbuf, "%s@%s", pw->pw_name, op->ldop_suffixes[ 0 ] ); + i = snprintf( tmpbuf, sizeof( tmpbuf ), "%s@%s", pw->pw_name, op->ldop_suffixes[ 0 ] ); + } + + if ( i < 0 || i >= sizeof( tmpbuf ) ) { + return NULL; } + + entry = (struct ldentry *) ecalloc( 1, sizeof( struct ldentry )); entry->lde_dn = estrdup( tmpbuf ); /* diff --git a/servers/slapd/slapi/plugin.c b/servers/slapd/slapi/plugin.c index da0085ca50..ecaf0fa697 100644 --- a/servers/slapd/slapi/plugin.c +++ b/servers/slapd/slapi/plugin.c @@ -722,7 +722,11 @@ slapi_int_plugin_unparse( slapi_pblock_get( pp, SLAPI_X_CONFIG_ARGV, &argv ); if ( argv == NULL ) /* could be dynamic plugin */ continue; - idx.bv_len = sprintf( idx.bv_val, "{%d}", i ); + idx.bv_len = snprintf( idx.bv_val, sizeof( ibuf ), "{%d}", i ); + if ( idx.bv_len >= sizeof( ibuf ) ) { + /* FIXME: just truncating by now */ + idx.bv_len = sizeof( ibuf ) - 1; + } bv.bv_len = idx.bv_len; for (j=1; argv[j]; j++) { bv.bv_len += strlen(argv[j]); diff --git a/servers/slapd/slapi/slapi_utils.c b/servers/slapd/slapi/slapi_utils.c index 6c37e3b7f4..d24be09b82 100644 --- a/servers/slapd/slapi/slapi_utils.c +++ b/servers/slapd/slapi/slapi_utils.c @@ -3216,21 +3216,25 @@ LDAP *slapi_ldap_init( char *ldaphost, int ldapport, int secure, int shared ) int rc; size = sizeof("ldap:///"); - if ( secure ) + if ( secure ) { size++; + } size += strlen( ldaphost ); - if ( ldapport != 0 ) + if ( ldapport != 0 ) { size += 32; + } url = slapi_ch_malloc( size ); if ( ldapport != 0 ) { - sprintf( url, "ldap%s://%s:%d/", ( secure ? "s" : "" ), ldaphost, ldapport ); + rc = snprintf( url, size, "ldap%s://%s:%d/", ( secure ? "s" : "" ), ldaphost, ldapport ); } else { - sprintf( url, "ldap%s://%s/", ( secure ? "s" : "" ), ldaphost ); + rc = snprintf( url, size, "ldap%s://%s/", ( secure ? "s" : "" ), ldaphost ); } - rc = ldap_initialize( &ld, url ); + if ( rc > 0 && rc < size ) { + rc = ldap_initialize( &ld, url ); + } slapi_ch_free_string( &url );