From a937ce2d8545568c1de3ba0370fd621ffcbdd8ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 8 Sep 2020 15:13:44 +0200 Subject: [PATCH] shared/libcrypt-util: use libcrypt_ra() 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 | 13 ++++++++----- src/shared/libcrypt-util.h | 5 ++++- src/test/meson.build | 4 ++++ src/test/test-libcrypt-util.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 src/test/test-libcrypt-util.c diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c index d19bcf1d8a..cb12cd65f3 100644 --- a/src/shared/libcrypt-util.c +++ b/src/shared/libcrypt-util.c @@ -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) diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h index b10be2f7d2..2f8c352ab3 100644 --- a/src/shared/libcrypt-util.h +++ b/src/shared/libcrypt-util.h @@ -18,5 +18,8 @@ #include 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); diff --git a/src/test/meson.build b/src/test/meson.build index 70450ea1c5..3e84e5a0d3 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -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 index 0000000000..94bd4172ea --- /dev/null +++ b/src/test/test-libcrypt-util.c @@ -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; +}