CVE-2020-10730: s4 dsdb vlv_pagination: Prevent repeat call of ldb_module_done

Check the return code from vlv_results, if it is not LDB_SUCCESS
ldb_module_done has already been called, and SHOULD NOT be called again.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364

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

1 files changed, 49 insertions, 12 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index 720b5e95638..b103bda5f52 100644
--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
@@ -387,7 +387,7 @@ static int vlv_calc_real_offset(int offset, int denominator, int n_entries)
    has been prepared earlier and saved -- or by vlv_search_callback() when a
    search has just been completed. */
 
-static int vlv_results(struct vlv_context *ac)
+static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares)
 {
 	struct ldb_vlv_resp_control *vlv;
 	unsigned int num_ctrls;
@@ -397,7 +397,9 @@ static int vlv_results(struct vlv_context *ac)
 	int target = 0;
 
 	if (ac->store == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	if (ac->store->first_ref) {
@@ -406,6 +408,10 @@ static int vlv_results(struct vlv_context *ac)
 		*/
 		ret = send_referrals(ac->store, ac->req);
 		if (ret != LDB_SUCCESS) {
+			/*
+			 * send_referrals will have called ldb_module_done
+			 * if there was an error.
+			 */
 			return ret;
 		}
 	}
@@ -419,14 +425,23 @@ static int vlv_results(struct vlv_context *ac)
 						    vlv_details,
 						    sort_details, &ret);
 			if (ret != LDB_SUCCESS) {
-				return ret;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 		} else {
 			target = vlv_calc_real_offset(vlv_details->match.byOffset.offset,
 						      vlv_details->match.byOffset.contentCount,
 						      ac->store->num_entries);
 			if (target == -1) {
-				return LDB_ERR_OPERATIONS_ERROR;
+				ret = LDB_ERR_OPERATIONS_ERROR;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 		}
 
@@ -462,12 +477,20 @@ static int vlv_results(struct vlv_context *ac)
 				}
 				continue;
 			} else if (ret != LDB_SUCCESS) {
-				return ret;
+				return ldb_module_done(
+					ac->req,
+					ac->controls,
+					ares->response,
+					ret);
 			}
 
 			ret = ldb_module_send_entry(ac->req, result->msgs[0],
 						    NULL);
 			if (ret != LDB_SUCCESS) {
+				/*
+				 * ldb_module_send_entry will have called
+				 * ldb_module_done if there was an error
+				 */
 				return ret;
 			}
 		}
@@ -488,7 +511,9 @@ static int vlv_results(struct vlv_context *ac)
 
 	ac->controls = talloc_array(ac, struct ldb_control *, num_ctrls + 1);
 	if (ac->controls == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 	ac->controls[num_ctrls] = NULL;
 
@@ -498,20 +523,26 @@ static int vlv_results(struct vlv_context *ac)
 
 	ac->controls[i] = talloc(ac->controls, struct ldb_control);
 	if (ac->controls[i] == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->oid = talloc_strdup(ac->controls[i],
 					     LDB_CONTROL_VLV_RESP_OID);
 	if (ac->controls[i]->oid == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 
 	ac->controls[i]->critical = 0;
 
 	vlv = talloc(ac->controls[i], struct ldb_vlv_resp_control);
 	if (vlv == NULL) {
-		return LDB_ERR_OPERATIONS_ERROR;
+		ret = LDB_ERR_OPERATIONS_ERROR;
+		return ldb_module_done(
+			ac->req, ac->controls, ares->response, ret);
 	}
 	ac->controls[i]->data = vlv;
 
@@ -600,7 +631,13 @@ static int vlv_search_callback(struct ldb_request *req, struct ldb_reply *ares)
 		store->result_array_size = store->num_entries;
 
 		ac->store->controls = talloc_move(ac->store, &ares->controls);
-		ret = vlv_results(ac);
+		ret = vlv_results(ac, ares);
+		if (ret != LDB_SUCCESS) {
+			/* vlv_results will have called ldb_module_done
+			 * if there was an error.
+			 */
+			return ret;
+		}
 		return ldb_module_done(ac->req, ac->controls,
 					ares->response, ret);
 	}
@@ -845,9 +882,9 @@ static int vlv_search(struct ldb_module *module, struct ldb_request *req)
 			return ret;
 		}
 
-		ret = vlv_results(ac);
+		ret = vlv_results(ac, NULL);
 		if (ret != LDB_SUCCESS) {
-			return ldb_module_done(req, NULL, NULL, ret);
+			return ret;
 		}
 		return ldb_module_done(req, ac->controls, NULL,
 				       LDB_SUCCESS);