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>

diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c
index 720b5e95638c92b401bf7cf7c2d2c324e22d2ef4..b103bda5f520534110908cdd90dab4a7b8529b7f 100644 (file)
--- 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);