From b6f21455e1662ee8768189692f88f959c804917d Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 28 Nov 2013 06:50:01 +1300 Subject: [PATCH] CVE-2013-4496:Revert remainder of ce895609b04380bfc41e4f8fddc84bd2f9324340 Part of this was removed when ChangePasswordUser was unimplemented, but remove the remainder of this flawed commit. Fully check the password first, as extract_pw_from_buffer() already does a partial check of the password because it needs a correct old password to correctly decrypt the length. Andrew Bartlett Bug: https://bugzilla.samba.org/show_bug.cgi?id=10245 Signed-off-by: Andrew Bartlett Reviewed-by: Andreas Schneider Reviewed-by: Stefan Metzmacher --- source4/rpc_server/samr/samr_password.c | 69 +++++++++++++------------ 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 9d6c9212f47..685a8e7864a 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -142,6 +142,9 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, E_deshash(new_pass, new_lm_hash); E_old_pw_hash(new_lm_hash, lm_pwd->hash, lm_verifier.hash); + if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) { + return NT_STATUS_WRONG_PASSWORD; + } /* Connect to a SAMDB with user privileges for the password change */ sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, @@ -173,11 +176,6 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, return status; } - if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) { - ldb_transaction_cancel(sam_ctx); - return NT_STATUS_WRONG_PASSWORD; - } - /* And this confirms it in a transaction commit */ ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { @@ -267,33 +265,8 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, goto failed; } - /* Connect to a SAMDB with user privileges for the password change */ - sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - dce_call->conn->auth_state.session_info, 0); - if (sam_ctx == NULL) { - return NT_STATUS_INVALID_SYSTEM_SERVICE; - } - - ret = ldb_transaction_start(sam_ctx); - if (ret != LDB_SUCCESS) { - DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); - return NT_STATUS_TRANSACTION_ABORTED; - } - - /* Performs the password modification. We pass the old hashes read out - * from the database since they were already checked against the user- - * provided ones. */ - status = samdb_set_password(sam_ctx, mem_ctx, - user_dn, NULL, - &new_password, - NULL, NULL, - lm_pwd, nt_pwd, /* this is a user password change */ - &reason, - &dominfo); - - if (!NT_STATUS_IS_OK(status)) { - ldb_transaction_cancel(sam_ctx); + if (r->in.nt_verifier == NULL) { + status = NT_STATUS_WRONG_PASSWORD; goto failed; } @@ -302,7 +275,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, E_old_pw_hash(new_nt_hash, nt_pwd->hash, nt_verifier.hash); if (memcmp(nt_verifier.hash, r->in.nt_verifier->hash, 16) != 0) { - ldb_transaction_cancel(sam_ctx); status = NT_STATUS_WRONG_PASSWORD; goto failed; } @@ -322,13 +294,42 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, E_deshash(new_pass, new_lm_hash); E_old_pw_hash(new_nt_hash, lm_pwd->hash, lm_verifier.hash); if (memcmp(lm_verifier.hash, r->in.lm_verifier->hash, 16) != 0) { - ldb_transaction_cancel(sam_ctx); status = NT_STATUS_WRONG_PASSWORD; goto failed; } } } + /* Connect to a SAMDB with user privileges for the password change */ + sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, + dce_call->conn->dce_ctx->lp_ctx, + dce_call->conn->auth_state.session_info, 0); + if (sam_ctx == NULL) { + return NT_STATUS_INVALID_SYSTEM_SERVICE; + } + + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_TRANSACTION_ABORTED; + } + + /* Performs the password modification. We pass the old hashes read out + * from the database since they were already checked against the user- + * provided ones. */ + status = samdb_set_password(sam_ctx, mem_ctx, + user_dn, NULL, + &new_password, + NULL, NULL, + lm_pwd, nt_pwd, /* this is a user password change */ + &reason, + &dominfo); + + if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); + goto failed; + } + /* And this confirms it in a transaction commit */ ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { -- 2.34.1