From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 4 Apr 2018 02:00:57 +0000 (+1200)
Subject: ldb_tdb: A more robust check for if we can fit the index string in
X-Git-Tag: ldb-1.4.0~707
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=556466e7e3bed0edd8cd15e58dd4380f13c14364;p=thirdparty%2Fsamba.git

ldb_tdb: A more robust check for if we can fit the index string in

This avoids magic numbers and also is careful against overflow
from a long attr_for_dn.

This is done as a distinct commit to make the previous behaviour
change more clear, and to show that this does not change the
calculations, only improves the overflow check.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---

diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index ed28c4f642f..59434f3ba99 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -842,25 +842,18 @@ static struct ldb_dn *ltdb_index_key(struct ldb_context *ldb,
 	const char *attr_for_dn = NULL;
 	int r;
 	bool should_b64_encode;
-	unsigned max_key_length = ltdb_max_key_length(ltdb);
-	unsigned key_len = 0;
-	unsigned attr_len = 0;
-	unsigned indx_len = 0;
-	unsigned frmt_len = 0;
-
-	if (max_key_length < 4) {
-		ldb_asprintf_errstring(
-			ldb,
-			__location__ ": max_key_length of (%u) < 4",
-			max_key_length);
-		return NULL;
-	}
-	/*
-	 * ltdb_key_dn() makes something 4 bytes longer, it adds a leading
-	 * "DN=" and a trailing string terninator
-	 */
-	max_key_length -= 4;
 
+	unsigned int max_key_length = ltdb_max_key_length(ltdb);
+	size_t key_len = 0;
+	size_t attr_len = 0;
+	const size_t indx_len = sizeof(LTDB_INDEX) - 1;
+	unsigned frmt_len = 0;
+	const size_t additional_key_length = 4;
+	unsigned int num_separators = 3; /* Estimate for overflow check */
+	const size_t min_data = 1;
+	const size_t min_key_length = additional_key_length
+		+ indx_len + num_separators + min_data;
+	
 	if (attr[0] == '@') {
 		attr_for_dn = attr;
 		v = *value;
@@ -897,7 +890,29 @@ static struct ldb_dn *ltdb_index_key(struct ldb_context *ldb,
 		}
 	}
 	attr_len = strlen(attr_for_dn);
-	indx_len = strlen(LTDB_INDEX);
+
+	/*
+	 * Check if there is any hope this will fit into the DB.
+	 * Overflow here is not actually critical the code below
+	 * checks again to make the printf and the DB does another
+	 * check for too long keys
+	 */
+	if (max_key_length - attr_len < min_key_length) {
+		ldb_asprintf_errstring(
+			ldb,
+			__location__ ": max_key_length "
+			"is too small (%u) < (%u)",
+			max_key_length,
+			(unsigned)(min_key_length + attr_len));
+		talloc_free(attr_folded);
+		return NULL;
+	}
+	
+	/*
+	 * ltdb_key_dn() makes something 4 bytes longer, it adds a leading
+	 * "DN=" and a trailing string terminator
+	 */
+	max_key_length -= additional_key_length;
 
 	/*
 	 * We do not base 64 encode a DN in a key, it has already been
@@ -922,17 +937,20 @@ static struct ldb_dn *ltdb_index_key(struct ldb_context *ldb,
 	}
 
 	if (should_b64_encode) {
-		unsigned vstr_len = 0;
+		size_t vstr_len = 0;
 		char *vstr = ldb_base64_encode(ldb, (char *)v.data, v.length);
-		unsigned num_separators = 3;
 		if (!vstr) {
 			talloc_free(attr_folded);
 			return NULL;
 		}
 		vstr_len = strlen(vstr);
+		/* 
+		 * Overflow here is not critical as we only use this
+		 * to choose the printf truncation
+		 */
 		key_len = num_separators + indx_len + attr_len + vstr_len;
 		if (key_len > max_key_length) {
-			unsigned excess = key_len - max_key_length;
+			size_t excess = key_len - max_key_length;
 			frmt_len = vstr_len - excess;
 			*truncation = KEY_TRUNCATED;
 			/*
@@ -957,10 +975,16 @@ static struct ldb_dn *ltdb_index_key(struct ldb_context *ldb,
 		}
 		talloc_free(vstr);
 	} else {
-		unsigned num_separators = 2;
+		/* Only need two seperators */
+		num_separators = 2;
+		
+		/* 
+		 * Overflow here is not critical as we only use this
+		 * to choose the printf truncation
+		 */
 		key_len = num_separators + indx_len + attr_len + (int)v.length;
 		if (key_len > max_key_length) {
-			unsigned excess = key_len - max_key_length;
+			size_t excess = key_len - max_key_length;
 			frmt_len = v.length - excess;
 			*truncation = KEY_TRUNCATED;
 			/*