CVE-2021-20251 s4:dsdb: Update bad password count inside transaction

Previously, there was a gap between calling dsdb_update_bad_pwd_count()
and dsdb_module_modify() where no transaction was in effect. Another
process could slip in and modify badPwdCount, only for our update to
immediately overwrite it. Doing the update inside the transaction will
help for the following commit when we make it atomic.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index e90482df63ea5929abdb1bd84201b9ad90771649..b9f7fb77e6f151f6ab3e6ab305907339a79be286 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -2733,24 +2733,6 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io
        int ret; /* The errors we will actually return */
        int dbg_ret; /* The errors we can only complain about in logs */
 
-       /* PSO search result is optional (NULL if no PSO applies) */
-       if (io->ac->pso_res != NULL) {
-               pso_msg = io->ac->pso_res->message;
-       }
-
-       status = dsdb_update_bad_pwd_count(io->ac, ldb,
-                                          io->ac->search_res->message,
-                                          io->ac->dom_res->message,
-                                          pso_msg,
-                                          &mod_msg);
-       if (!NT_STATUS_IS_OK(status)) {
-               goto done;
-       }
-
-       if (mod_msg == NULL) {
-               goto done;
-       }
-
        /*
         * OK, horrible semantics ahead.
         *
@@ -2793,6 +2775,24 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io
                goto done;
        }
 
+       /* PSO search result is optional (NULL if no PSO applies) */
+       if (io->ac->pso_res != NULL) {
+               pso_msg = io->ac->pso_res->message;
+       }
+
+       status = dsdb_update_bad_pwd_count(io->ac, ldb,
+                                          io->ac->search_res->message,
+                                          io->ac->dom_res->message,
+                                          pso_msg,
+                                          &mod_msg);
+       if (!NT_STATUS_IS_OK(status)) {
+               goto end_transaction;
+       }
+
+       if (mod_msg == NULL) {
+               goto end_transaction;
+       }
+
        dbg_ret = dsdb_module_modify(io->ac->module, mod_msg,
                                 DSDB_FLAG_NEXT_MODULE,
                                 io->ac->req);
@@ -2806,6 +2806,7 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io
                 */
        }
 
+end_transaction:
        dbg_ret = ldb_next_end_trans(io->ac->module);
        if (dbg_ret != LDB_SUCCESS) {
                ldb_debug(ldb, LDB_DEBUG_ERROR,