[PATCHES] avoid memory errors with ldb client cookies

[Douglas Bagnall]

hi,

Ldb has a function int ldb_base64_decode(char *) that decodes a base64
string in place and returns the new length (it is always shorter). If
the string is not base64, it returns -1. Callers of this function in
lib/ldb/common/ldb_controls.c were using the returned value in a
talloc_memdup, where the -1 would convert to the largest size_t the
machine could imagine. That is very unlikely to be successful.

AFAICT, this is only used in lib/ldb/tools/ldb*.c and in Python code.

The second patch checks if the same talloc_memdup()s fail in mundane ways.

Douglas
-------------- next part --------------
From 98660628eb8614ccb296163a0f65181d19bfcd28 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 16 Mar 2016 12:46:12 +1300
Subject: [PATCH 1/2] ldb client controls: avoid talloc_memdup(x, y,
 (size_t)-1);

ldb_base64_decode() returns -1 if a string can't be parsed as base64,
and this is not the kind of value you want to use in talloc_memdup().

In these cases it can happen innocently if the strings are truncated
to fit in their buffers.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 lib/ldb/common/ldb_controls.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/lib/ldb/common/ldb_controls.c b/lib/ldb/common/ldb_controls.c
index 4fd3c69..ba813ea 100644
--- a/lib/ldb/common/ldb_controls.c
+++ b/lib/ldb/common/ldb_controls.c
@@ -510,8 +510,16 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 			control->match.byOffset.contentCount = cc;
 		}
 		if (ctxid[0]) {
-			control->ctxid_len = ldb_base64_decode(ctxid);
-			control->contextId = talloc_memdup(control, ctxid, control->ctxid_len);
+			int len = ldb_base64_decode(ctxid);
+			if (len < 0) {
+				ldb_set_errstring(ldb,
+						  "invalid VLV context_id\n");
+				talloc_free(ctrl);
+				return NULL;
+			}
+			control->ctxid_len = len;
+			control->contextId = talloc_memdup(control, ctxid,
+							   control->ctxid_len);
 		} else {
 			control->ctxid_len = 0;
 			control->contextId = NULL;
@@ -555,7 +563,14 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 		control->flags = flags;
 		control->max_attributes = max_attrs;
 		if (*cookie) {
-			control->cookie_len = ldb_base64_decode(cookie);
+			int len = ldb_base64_decode(cookie);
+			if (len < 0) {
+				ldb_set_errstring(ldb,
+						  "invalid dirsync cookie\n");
+				talloc_free(ctrl);
+				return NULL;
+			}
+			control->cookie_len = len;
 			control->cookie = (char *)talloc_memdup(control, cookie, control->cookie_len);
 		} else {
 			control->cookie = NULL;
@@ -600,7 +615,15 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 		control->flags = flags;
 		control->max_attributes = max_attrs;
 		if (*cookie) {
-			control->cookie_len = ldb_base64_decode(cookie);
+			int len = ldb_base64_decode(cookie);
+			if (len < 0) {
+				ldb_set_errstring(ldb,
+						  "invalid dirsync_ex cookie"
+						  " (probably too long)\n");
+				talloc_free(ctrl);
+				return NULL;
+			}
+			control->cookie_len = len;
 			control->cookie = (char *)talloc_memdup(control, cookie, control->cookie_len);
 		} else {
 			control->cookie = NULL;
-- 
2.5.0


From 19516ea91e66519b6ffa4b05945384faa3f7b404 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 17 Mar 2016 10:33:56 +1300
Subject: [PATCH 2/2] ldb client controls: don't ignore failed memdup

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 lib/ldb/common/ldb_controls.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/ldb/common/ldb_controls.c b/lib/ldb/common/ldb_controls.c
index ba813ea..63d5929 100644
--- a/lib/ldb/common/ldb_controls.c
+++ b/lib/ldb/common/ldb_controls.c
@@ -520,6 +520,9 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 			control->ctxid_len = len;
 			control->contextId = talloc_memdup(control, ctxid,
 							   control->ctxid_len);
+			if (control->contextId == NULL) {
+				return NULL;
+			}
 		} else {
 			control->ctxid_len = 0;
 			control->contextId = NULL;
@@ -572,6 +575,9 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 			}
 			control->cookie_len = len;
 			control->cookie = (char *)talloc_memdup(control, cookie, control->cookie_len);
+			if (control->cookie == NULL) {
+				return NULL;
+			}
 		} else {
 			control->cookie = NULL;
 			control->cookie_len = 0;
@@ -625,6 +631,9 @@ struct ldb_control *ldb_parse_control_from_string(struct ldb_context *ldb, TALLO
 			}
 			control->cookie_len = len;
 			control->cookie = (char *)talloc_memdup(control, cookie, control->cookie_len);
+			if (control->cookie == NULL) {
+				return NULL;
+			}
 		} else {
 			control->cookie = NULL;
 			control->cookie_len = 0;
-- 
2.5.0