]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
shared/libcrypt-util: use libcrypt_ra()
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 8 Sep 2020 13:13:44 +0000 (15:13 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 15 Sep 2020 07:30:56 +0000 (09:30 +0200)
This lets the libc/xcrypt allocate as much storage area as it needs.
Should fix #16965:

testsuite-46.sh[74]: ==74==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f3e972e1080 at pc 0x7f3e9be8deed bp 0x7ffce4f28530 sp 0x7ffce4f27ce0
testsuite-46.sh[74]: WRITE of size 131232 at 0x7f3e972e1080 thread T0
testsuite-46.sh[74]:     #0 0x7f3e9be8deec  (/usr/lib/clang/10.0.1/lib/linux/libclang_rt.asan-x86_64.so+0x9feec)
testsuite-46.sh[74]:     #1 0x559cd05a6412 in user_record_make_hashed_password /systemd-meson-build/../build/src/home/user-record-util.c:818:21
testsuite-46.sh[74]:     #2 0x559cd058fb03 in create_home /systemd-meson-build/../build/src/home/homectl.c:1112:29
testsuite-46.sh[74]:     #3 0x7f3e9b5b3058 in dispatch_verb /systemd-meson-build/../build/src/shared/verbs.c:103:24
testsuite-46.sh[74]:     #4 0x559cd058c101 in run /systemd-meson-build/../build/src/home/homectl.c:3325:16
testsuite-46.sh[74]:     #5 0x559cd058c00a in main /systemd-meson-build/../build/src/home/homectl.c:3328:1
testsuite-46.sh[74]:     #6 0x7f3e9a88b151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
testsuite-46.sh[74]:     #7 0x559cd0583e7d in _start (/usr/bin/homectl+0x24e7d)
testsuite-46.sh[74]: Address 0x7f3e972e1080 is located in stack of thread T0 at offset 32896 in frame
testsuite-46.sh[74]:     #0 0x559cd05a60df in user_record_make_hashed_password /systemd-meson-build/../build/src/home/user-record-util.c:789
testsuite-46.sh[74]:   This frame has 6 object(s):
testsuite-46.sh[74]:     [32, 40) 'priv' (line 790)
testsuite-46.sh[74]:     [64, 72) 'np' (line 791)
testsuite-46.sh[74]:     [96, 104) 'salt' (line 809)
testsuite-46.sh[74]:     [128, 32896) 'cd' (line 810)
testsuite-46.sh[74]:     [33152, 33168) '.compoundliteral' <== Memory access at offset 32896 partially underflows this variable
testsuite-46.sh[74]:     [33184, 33192) 'new_array' (line 832) <== Memory access at offset 32896 partially underflows this variable
testsuite-46.sh[74]: HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
testsuite-46.sh[74]:       (longjmp and C++ exceptions *are* supported)
testsuite-46.sh[74]: SUMMARY: AddressSanitizer: stack-buffer-overflow (/usr/lib/clang/10.0.1/lib/linux/libclang_rt.asan-x86_64.so+0x9feec)

It seems 'struct crypt_data' is 32896 bytes, but libclang_rt wants more, at least 33168?

src/shared/libcrypt-util.c
src/shared/libcrypt-util.h
src/test/meson.build
src/test/test-libcrypt-util.c [new file with mode: 0644]

index d19bcf1d8abbf19ccceca02bda82002c32775498..cb12cd65f37f3ef36c7921e4e4b722683dda694e 100644 (file)
@@ -8,6 +8,7 @@
 #include "libcrypt-util.h"
 #include "log.h"
 #include "macro.h"
+#include "memory-util.h"
 #include "missing_stdlib.h"
 #include "random-util.h"
 #include "string-util.h"
@@ -75,21 +76,23 @@ int make_salt(char **ret) {
 #endif
 }
 
-int hash_password(const char *password, char **ret) {
+int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret) {
         _cleanup_free_ char *salt = NULL;
+        _cleanup_(erase_and_freep) void *_cd_data = NULL;
         char *p;
-        struct crypt_data cd = {};
-        int r;
+        int r, _cd_size = 0;
+
+        assert(!!cd_data == !!cd_size);
 
         r = make_salt(&salt);
         if (r < 0)
                 return log_debug_errno(r, "Failed to generate salt: %m");
 
         errno = 0;
-        p = crypt_r(password, salt, &cd);
+        p = crypt_ra(password, salt, cd_data ?: &_cd_data, cd_size ?: &_cd_size);
         if (!p)
                 return log_debug_errno(errno_or_else(SYNTHETIC_ERRNO(EINVAL)),
-                                       "crypt_r() failed: %m");
+                                       "crypt_ra() failed: %m");
 
         p = strdup(p);
         if (!p)
index b10be2f7d29e5ebd4d7b0ab019ae755639d45fce..2f8c352ab3710d261218e2753a1120b1d03bd8b7 100644 (file)
@@ -18,5 +18,8 @@
 #include <stdlib.h>
 
 int make_salt(char **ret);
-int hash_password(const char *password, char **ret);
+int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret);
+static inline int hash_password(const char *password, char **ret) {
+        return hash_password_full(password, NULL, NULL, ret);
+}
 bool looks_like_hashed_password(const char *s);
index 70450ea1c5a19f63688516b121832c44e99bfbd3..3e84e5a0d36ce1e5125dfcf0a7034e225584836b 100644 (file)
@@ -301,6 +301,10 @@ tests += [
          [],
          []],
 
+        [['src/test/test-libcrypt-util.c'],
+         [],
+         []],
+
         [['src/test/test-offline-passwd.c',
           'src/shared/offline-passwd.c',
           'src/shared/offline-passwd.h'],
diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
new file mode 100644 (file)
index 0000000..94bd417
--- /dev/null
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: LGPL-2.1+ */
+
+#include "strv.h"
+#include "tests.h"
+#include "libcrypt-util.h"
+
+static void test_hash_password_full(void) {
+        log_info("/* %s */", __func__);
+
+        _cleanup_free_ void *cd_data = NULL;
+        const char *i;
+        int cd_size = 0;
+
+        log_info("sizeof(struct crypt_data): %zu bytes", sizeof(struct crypt_data));
+
+        for (unsigned c = 0; c < 2; c++)
+                FOREACH_STRING(i, "abc123", "password", "s3cr3t") {
+                        _cleanup_free_ char *hashed;
+
+                        if (c == 0)
+                                assert_se(hash_password_full(i, &cd_data, &cd_size, &hashed) == 0);
+                        else
+                                assert_se(hash_password_full(i, NULL, NULL, &hashed) == 0);
+                        log_debug("\"%s\" → \"%s\"", i, hashed);
+                        log_info("crypt_r[a] buffer size: %i bytes", cd_size);
+                }
+}
+
+int main(int argc, char *argv[]) {
+        test_setup_logging(LOG_DEBUG);
+
+        test_hash_password_full();
+
+        return 0;
+}