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>

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 e521ed09999d68900cc10da2ad0de4740ad5878a..3ebec827404ce4bae0709fe1922d144fbb534887 100644 (file)
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -2036,21 +2036,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;
        }
 
@@ -2058,6 +2073,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;
@@ -2120,7 +2136,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);
                        cps_stdin = -1;
                        TALLOC_FREE(password_script);