From c2918581996b58c3e2a2416e6dd693978becd3a2 Mon Sep 17 00:00:00 2001 From: Matthias Dieter Wallnöfer Date: Thu, 14 Oct 2010 17:01:39 +0200 Subject: s4:dsdb - fix unsigned integer save problems using the "%u" specifier The issue here is that we have not yet first cast to int32_t explicitly, before we cast to an signed int to printf() into the %d or cast to a int64_t before we then cast to a long long to printf into a %lld. There are *no* unsigned integers in Active Directory LDAP, even the RID allocations and ms-DS-Secondary-KrbTgt-Number are *signed* quantities. (See the schema, and the syntax definitions in schema_syntax.c). The failure has been detected by Matthieu Patou on the buildfarm host "tridge" due to a malformed "groupType" attribute. The solution is to use the "%d" specifier. Either to use it directly - or better (when possible) use the call "samdb_msg_add_uint" (which encapsulates it). This patch changes such problematic situations. --- source4/dsdb/samdb/ldb_modules/acl.c | 4 ++-- source4/dsdb/samdb/ldb_modules/instancetype.c | 2 +- source4/dsdb/samdb/ldb_modules/operational.c | 10 +++++++++- source4/dsdb/samdb/ldb_modules/rootdse.c | 17 +++++++++-------- source4/dsdb/samdb/ldb_modules/samldb.c | 15 ++++++++++----- source4/libnet/libnet_become_dc.c | 5 +++-- source4/libnet/libnet_unbecome_dc.c | 5 +++-- source4/rpc_server/lsa/dcesrv_lsa.c | 24 ++++++++++++------------ 8 files changed, 49 insertions(+), 33 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 660b4df818..3e302ef4ef 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -425,8 +425,8 @@ static int acl_sDRightsEffective(struct ldb_module *module, flags |= SECINFO_SACL; } } - ldb_msg_add_fmt(msg, "sDRightsEffective", "%u", flags); - return LDB_SUCCESS; + return samdb_msg_add_uint(ldb_module_get_ctx(module), msg, msg, + "sDRightsEffective", flags); } static int acl_add(struct ldb_module *module, struct ldb_request *req) diff --git a/source4/dsdb/samdb/ldb_modules/instancetype.c b/source4/dsdb/samdb/ldb_modules/instancetype.c index 5032462196..a728502449 100644 --- a/source4/dsdb/samdb/ldb_modules/instancetype.c +++ b/source4/dsdb/samdb/ldb_modules/instancetype.c @@ -162,7 +162,7 @@ static int instancetype_add(struct ldb_module *module, struct ldb_request *req) */ instanceType = INSTANCE_TYPE_WRITE; - ret = ldb_msg_add_fmt(msg, "instanceType", "%u", instanceType); + ret = samdb_msg_add_uint(ldb, msg, msg, "instanceType", instanceType); if (ret != LDB_SUCCESS) { return ret; } diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c index 633fd8d28d..5a5b5e903a 100644 --- a/source4/dsdb/samdb/ldb_modules/operational.c +++ b/source4/dsdb/samdb/ldb_modules/operational.c @@ -454,6 +454,7 @@ static int construct_msds_keyversionnumber(struct ldb_module *module, enum ndr_err_code ndr_err; const struct ldb_val *omd_value; struct replPropertyMetaDataBlob *omd; + int ret; omd_value = ldb_msg_find_ldb_val(msg, "replPropertyMetaData"); if (!omd_value) { @@ -486,7 +487,14 @@ static int construct_msds_keyversionnumber(struct ldb_module *module, } for (i=0; ictr.ctr1.count; i++) { if (omd->ctr.ctr1.array[i].attid == DRSUAPI_ATTRIBUTE_unicodePwd) { - ldb_msg_add_fmt(msg, "msDS-KeyVersionNumber", "%u", omd->ctr.ctr1.array[i].version); + ret = samdb_msg_add_uint(ldb_module_get_ctx(module), + msg, msg, + "msDS-KeyVersionNumber", + omd->ctr.ctr1.array[i].version); + if (ret != LDB_SUCCESS) { + talloc_free(omd); + return ret; + } break; } } diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c index 7334bf3bc5..a51785e64d 100644 --- a/source4/dsdb/samdb/ldb_modules/rootdse.c +++ b/source4/dsdb/samdb/ldb_modules/rootdse.c @@ -285,8 +285,9 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_message *ms uint64_t seq_num; int ret = ldb_sequence_number(ldb, LDB_SEQ_HIGHEST_SEQ, &seq_num); if (ret == LDB_SUCCESS) { - if (ldb_msg_add_fmt(msg, "highestCommittedUSN", - "%llu", (unsigned long long)seq_num) != LDB_SUCCESS) { + if (samdb_msg_add_uint64(ldb, msg, msg, + "highestCommittedUSN", + seq_num) != LDB_SUCCESS) { goto failed; } } @@ -300,8 +301,8 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_message *ms n++; } - if (ldb_msg_add_fmt(msg, "dsSchemaAttrCount", - "%u", n) != LDB_SUCCESS) { + if (samdb_msg_add_uint(ldb, msg, msg, "dsSchemaAttrCount", + n) != LDB_SUCCESS) { goto failed; } } @@ -314,15 +315,15 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_message *ms n++; } - if (ldb_msg_add_fmt(msg, "dsSchemaClassCount", - "%u", n) != LDB_SUCCESS) { + if (samdb_msg_add_uint(ldb, msg, msg, "dsSchemaClassCount", + n) != LDB_SUCCESS) { goto failed; } } if (schema && do_attribute_explicit(attrs, "dsSchemaPrefixCount")) { - if (ldb_msg_add_fmt(msg, "dsSchemaPrefixCount", - "%u", schema->prefixmap->length) != LDB_SUCCESS) { + if (samdb_msg_add_uint(ldb, msg, msg, "dsSchemaPrefixCount", + schema->prefixmap->length) != LDB_SUCCESS) { goto failed; } } diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index ff110b7402..9d4f3b8672 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -293,7 +293,8 @@ found: return ldb_operr(ldb); } - ret = ldb_msg_add_fmt(ac->msg, "msDS-SecondaryKrbTgtNumber", "%u", krbtgt_number); + ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg, + "msDS-SecondaryKrbTgtNumber", krbtgt_number); if (ret != LDB_SUCCESS) { return ldb_operr(ldb); } @@ -757,6 +758,7 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac) struct ldb_message_element *el, *el2; enum sid_generator sid_generator; struct dom_sid *sid; + const char *tempstr; int ret; /* make sure that "sAMAccountType" is not specified */ @@ -791,9 +793,10 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac) if (strcmp(ac->type, "user") == 0) { /* Step 1.2: Default values */ + tempstr = talloc_asprintf(ac->msg, "%d", UF_NORMAL_ACCOUNT); + if (tempstr == NULL) return ldb_operr(ldb); ret = samdb_find_or_add_attribute(ldb, ac->msg, - "userAccountControl", - talloc_asprintf(ac->msg, "%d", UF_NORMAL_ACCOUNT)); + "userAccountControl", tempstr); if (ret != LDB_SUCCESS) return ret; ret = samdb_find_or_add_attribute(ldb, ac->msg, "badPwdCount", "0"); @@ -894,9 +897,11 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac) } else if (strcmp(ac->type, "group") == 0) { /* Step 2.2: Default values */ + tempstr = talloc_asprintf(ac->msg, "%d", + GTYPE_SECURITY_GLOBAL_GROUP); + if (tempstr == NULL) return ldb_operr(ldb); ret = samdb_find_or_add_attribute(ldb, ac->msg, - "groupType", - talloc_asprintf(ac->msg, "%d", GTYPE_SECURITY_GLOBAL_GROUP)); + "groupType", tempstr); if (ret != LDB_SUCCESS) return ret; /* Step 2.3: "groupType" -> "sAMAccountType" */ diff --git a/source4/libnet/libnet_become_dc.c b/source4/libnet/libnet_become_dc.c index 9da2515134..157ddce703 100644 --- a/source4/libnet/libnet_become_dc.c +++ b/source4/libnet/libnet_become_dc.c @@ -3016,8 +3016,9 @@ static NTSTATUS becomeDC_ldap2_modify_computer(struct libnet_BecomeDC_state *s) msg->dn = ldb_dn_new(msg, s->ldap2.ldb, s->dest_dsa.computer_dn_str); NT_STATUS_HAVE_NO_MEMORY(msg->dn); - ret = ldb_msg_add_fmt(msg, "userAccountControl", "%u", user_account_control); - if (ret != 0) { + ret = samdb_msg_add_uint(s->ldap2.ldb, msg, msg, "userAccountControl", + user_account_control); + if (ret != LDB_SUCCESS) { talloc_free(msg); return NT_STATUS_NO_MEMORY; } diff --git a/source4/libnet/libnet_unbecome_dc.c b/source4/libnet/libnet_unbecome_dc.c index d66c4be093..bc96c4ac3e 100644 --- a/source4/libnet/libnet_unbecome_dc.c +++ b/source4/libnet/libnet_unbecome_dc.c @@ -431,8 +431,9 @@ static NTSTATUS unbecomeDC_ldap_modify_computer(struct libnet_UnbecomeDC_state * msg->dn = ldb_dn_new(msg, s->ldap.ldb, s->dest_dsa.computer_dn_str); NT_STATUS_HAVE_NO_MEMORY(msg->dn); - ret = ldb_msg_add_fmt(msg, "userAccountControl", "%u", user_account_control); - if (ret != 0) { + ret = samdb_msg_add_uint(s->ldap.ldb, msg, msg, "userAccountControl", + user_account_control); + if (ret != LDB_SUCCESS) { talloc_free(msg); return NT_STATUS_NO_MEMORY; } diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c index d2339c0e5f..4014ae0742 100644 --- a/source4/rpc_server/lsa/dcesrv_lsa.c +++ b/source4/rpc_server/lsa/dcesrv_lsa.c @@ -814,8 +814,8 @@ static NTSTATUS add_trust_user(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - ret = ldb_msg_add_fmt(msg, "userAccountControl", "%u", - UF_INTERDOMAIN_TRUST_ACCOUNT); + ret = samdb_msg_add_uint(sam_ldb, msg, msg, "userAccountControl", + UF_INTERDOMAIN_TRUST_ACCOUNT); if (ret != LDB_SUCCESS) { return NT_STATUS_NO_MEMORY; } @@ -1419,6 +1419,7 @@ static NTSTATUS get_tdo(struct ldb_context *sam, TALLOC_CTX *mem_ctx, } static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ldb, struct ldb_message *orig, struct ldb_message *dest, const char *attribute, @@ -1427,7 +1428,6 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx, { const struct ldb_val *orig_val; uint32_t orig_uint = 0; - char *str_val; int flags = 0; int ret; @@ -1455,11 +1455,7 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - str_val = talloc_asprintf(mem_ctx, "%u", value); - if (!str_val) { - return NT_STATUS_NO_MEMORY; - } - ret = ldb_msg_add_steal_string(dest, attribute, str_val); + ret = samdb_msg_add_uint(sam_ldb, dest, dest, attribute, value); if (ret != LDB_SUCCESS) { return NT_STATUS_NO_MEMORY; } @@ -1721,7 +1717,8 @@ static NTSTATUS setInfoTrustedDomain_base(struct dcesrv_call_state *dce_call, msg->dn = dom_msg->dn; if (posix_offset) { - nt_status = update_uint32_t_value(mem_ctx, dom_msg, msg, + nt_status = update_uint32_t_value(mem_ctx, p_state->sam_ldb, + dom_msg, msg, "trustPosixOffset", *posix_offset, NULL); if (!NT_STATUS_IS_OK(nt_status)) { @@ -1735,7 +1732,8 @@ static NTSTATUS setInfoTrustedDomain_base(struct dcesrv_call_state *dce_call, uint32_t tmp; int origtype; - nt_status = update_uint32_t_value(mem_ctx, dom_msg, msg, + nt_status = update_uint32_t_value(mem_ctx, p_state->sam_ldb, + dom_msg, msg, "trustDirection", info_ex->trust_direction, &origdir); @@ -1766,7 +1764,8 @@ static NTSTATUS setInfoTrustedDomain_base(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_PARAMETER; } - nt_status = update_uint32_t_value(mem_ctx, dom_msg, msg, + nt_status = update_uint32_t_value(mem_ctx, p_state->sam_ldb, + dom_msg, msg, "trustAttributes", info_ex->trust_attributes, &origattrs); @@ -1785,7 +1784,8 @@ static NTSTATUS setInfoTrustedDomain_base(struct dcesrv_call_state *dce_call, } if (enc_types) { - nt_status = update_uint32_t_value(mem_ctx, dom_msg, msg, + nt_status = update_uint32_t_value(mem_ctx, p_state->sam_ldb, + dom_msg, msg, "msDS-SupportedEncryptionTypes", *enc_types, NULL); if (!NT_STATUS_IS_OK(nt_status)) { -- cgit