CVE-2019-14833 dsdb: send full password to check password script

utf8_len represents the number of characters (not bytes) of the
password. If the password includes multi-byte characters it is required
to write the total number of bytes to the check password script.
Otherwise the last bytes of the password string would be ignored.

Therefore we rename utf8_len to be clear what it does and does
not represent.

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

Signed-off-by: Björn Baumbach <bb@sernet.de>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(master): Tue Oct 29 11:58:45 UTC 2019 on sn-devel-184

diff --git a/selftest/knownfail.d/unacceptable-passwords b/selftest/knownfail.d/unacceptable-passwords
deleted file mode 100644 (file)
index 75fa2fc..0000000
--- a/selftest/knownfail.d/unacceptable-passwords
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.samba_tool.user_check_password_script.samba.tests.samba_tool.user_check_password_script.UserCheckPwdTestCase.test_checkpassword_unacceptable\(chgdcpass:local\)
\ No newline at end of file

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 817fce6e17f037ed638203d9f12c06aacc795932..bad2ee7a494c60087928baa640380ba7f410f87d 100644 (file)
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -2041,21 +2041,36 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
                                                const uint32_t pwdProperties,
                                                const uint32_t minPwdLength)
 {
-       const char *utf8_pw = (const char *)utf8_blob->data;
-       size_t utf8_len = strlen_m(utf8_pw);
        char *password_script = NULL;
+       const char *utf8_pw = (const char *)utf8_blob->data;
+
+       /*
+        * This looks strange because it is.
+        *
+        * The check for the number of characters in the password
+        * should clearly not be against the byte length, or else a
+        * single UTF8 character would count for more than one.
+        *
+        * We have chosen to use the number of 16-bit units that the
+        * password encodes to as the measure of length.  This is not
+        * the same as the number of codepoints, if a password
+        * contains a character beyond the Basic Multilingual Plane
+        * (above 65535) it will count for more than one "character".
+        */
+
+       size_t password_characters_roughly = strlen_m(utf8_pw);
 
        /* checks if the "minPwdLength" property is satisfied */
-       if (minPwdLength > utf8_len) {
+       if (minPwdLength > password_characters_roughly) {
                return SAMR_VALIDATION_STATUS_PWD_TOO_SHORT;
        }
 
-       /* checks the password complexity */
+       /* We might not be asked to check the password complexity */
        if (!(pwdProperties & DOMAIN_PASSWORD_COMPLEX)) {
                return SAMR_VALIDATION_STATUS_SUCCESS;
        }
 
-       if (utf8_len == 0) {
+       if (password_characters_roughly == 0) {
                return SAMR_VALIDATION_STATUS_NOT_COMPLEX_ENOUGH;
        }
 
@@ -2063,6 +2078,7 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
        if (password_script != NULL && *password_script != '\0') {
                int check_ret = 0;
                int error = 0;
+               ssize_t nwritten = 0;
                struct tevent_context *event_ctx = NULL;
                struct tevent_req *req = NULL;
                int cps_stdin = -1;
@@ -2125,7 +2141,9 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 
                cps_stdin = samba_runcmd_export_stdin(req);
 
-               if (write(cps_stdin, utf8_pw, utf8_len) != utf8_len) {
+               nwritten = write(cps_stdin, utf8_blob->data,
+                                utf8_blob->length);
+               if (nwritten != utf8_blob->length) {
                        close(cps_stdin);
                        TALLOC_FREE(password_script);
                        TALLOC_FREE(event_ctx);