diff --git a/src/basic/log.c b/src/basic/log.c index 29702abd44..17ea26d8a4 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -726,7 +726,11 @@ static int write_to_journal( if (LOG_PRI(level) > log_target_max_level[LOG_TARGET_JOURNAL]) return 0; - iovec_len = MIN(6 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, iovec_len, 6)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); log_do_header(header, sizeof(header), level, error, file, line, func, object_field, object, extra_field, extra); @@ -1024,7 +1028,11 @@ int log_struct_internal( int r; bool fallback = false; - iovec_len = MIN(17 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, iovec_len, 17)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); /* If the journal is available do structured logging. @@ -1123,15 +1131,21 @@ int log_struct_iovec_internal( char header[LINE_MAX]; struct iovec *iovec; - size_t n = 0, iovec_len; + size_t n = 0, iovec_len, n_context_iovec, n_input_iovec_expanded; - iovec_len = MIN(1 + n_input_iovec * 2 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&n_input_iovec_expanded, n_input_iovec, 2) || + !MUL_SAFE(&n_context_iovec, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, 1, n_input_iovec_expanded) || + !ADD_SAFE(&iovec_len, iovec_len, n_context_iovec)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); log_do_header(header, sizeof(header), level, error, file, line, func, NULL, NULL, NULL, NULL); iovec[n++] = IOVEC_MAKE_STRING(header); - for (size_t i = 0; i < n_input_iovec; i++) { + for (size_t i = 0; i < n_input_iovec && n + 2 <= iovec_len; i++) { iovec[n++] = input_iovec[i]; iovec[n++] = IOVEC_MAKE_STRING("\n"); } diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 1dc90d538a..047a58ade5 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -357,6 +357,33 @@ static char *ascii_ellipsize_mem(const char *s, size_t old_length, size_t new_le return t; } +/* Walk backwards from 'end' (exclusive) to the start of the last complete UTF-8 character, without + * descending below 'start', validate it and return a pointer to its first byte, optionally decoding it + * into *ret_c. Returns NULL on a truncated or otherwise malformed sequence. */ +static const char* find_previous_unichar(const char *start, const char *end, char32_t *ret_c) { + const char *p; + int r; + + assert(start); + assert(end); + assert(end > start); + + for (p = end; p > start; ) { + p--; + if (((uint8_t) *p & 0xc0) != 0x80) /* Found a non-continuation byte, i.e. a character start. */ + break; + } + + r = utf8_encoded_valid_unichar(p, end - p); + if (r < 0 || p + r != end) + return NULL; + + if (ret_c) + assert_se(utf8_encoded_to_unichar(p, ret_c) == r); + + return p; +} + char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigned percent) { size_t x, k, len, len2; const char *i, *j; @@ -400,11 +427,13 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne continue; /* ANSI sequences don't take up any space in output */ } - char32_t c; - r = utf8_encoded_to_unichar(i, &c); + r = utf8_encoded_valid_unichar(i, s + old_length - i); if (r < 0) return NULL; + char32_t c; + assert_se(utf8_encoded_to_unichar(i, &c) == r); + int w = unichar_iswide(c) ? 2 : 1; if (k + w > x) break; @@ -431,9 +460,9 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne continue; } - tt = utf8_prev_char(t); - r = utf8_encoded_to_unichar(tt, &c); - if (r < 0) + /* Find the previous complete UTF-8 character inside the retained suffix. */ + tt = find_previous_unichar(i, t, &c); + if (!tt) return NULL; w = unichar_iswide(c) ? 2 : 1; @@ -451,11 +480,21 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne if (k >= new_length) { /* Make space for ellipsis, if required and possible. We know that the edge character is not * part of an ANSI sequence (because then we'd skip it). If the last character we looked at - * was wide, we don't need to make space. */ - if (j < s + old_length) - j = utf8_next_char(j); - else if (i > s) - i = utf8_prev_char(i); + * was wide, we don't need to make space. + * Move the edge by one complete UTF-8 character within the input slice. */ + if (j < s + old_length) { + r = utf8_encoded_valid_unichar(j, s + old_length - j); + if (r < 0) + return NULL; + + j += r; + } else if (i > s) { + const char *tt = find_previous_unichar(s, i, NULL); + if (!tt) + return NULL; + + i = tt; + } } len = i - s; diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 7b9bd0d71a..b18805fa3e 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -3702,7 +3702,7 @@ int home_passwd_luks( const PasswordCache *cache, /* the passwords acquired via PKCS#11/FIDO2 security tokens */ char **effective_passwords /* new passwords */) { - size_t volume_key_size, max_key_slots, n_effective; + size_t volume_key_size, max_key_slots, n_effective, n_occupied = 0; _cleanup_(erase_and_freep) void *volume_key = NULL; struct crypt_pbkdf_type good_pbkdf, minimal_pbkdf; const char *type; @@ -3742,44 +3742,97 @@ int home_passwd_luks( n_effective = strv_length(effective_passwords); + for (size_t i = 0; i < max_key_slots; i++) + switch (sym_crypt_keyslot_status(setup->crypt_device, i)) { + case CRYPT_SLOT_INACTIVE: + break; + case CRYPT_SLOT_ACTIVE: + case CRYPT_SLOT_ACTIVE_LAST: + case CRYPT_SLOT_UNBOUND: + n_occupied++; + break; + default: + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Failed to determine state of LUKS key slot %zu.", i); + } + + if (n_effective > max_key_slots - n_occupied) + return log_error_errno(SYNTHETIC_ERRNO(ENOSPC), + "Not enough free LUKS key slots (%zu required, %zu available).", + n_effective, max_key_slots - n_occupied); + build_good_pbkdf(&good_pbkdf, h); build_minimal_pbkdf(&minimal_pbkdf, h); - for (size_t i = 0; i < max_key_slots; i++) { - r = sym_crypt_keyslot_destroy(setup->crypt_device, i); - if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */ - return log_error_errno(r, "Failed to destroy LUKS password: %m"); + /* Rotate the LUKS key slots in two passes, so that at every on-disk state at least one valid key slot + * exists for each password the user currently holds. First add all new passwords into free key slots + * (purely additive, hence non-destructive), and only once that fully succeeded destroy the old slots. + * If an add fails (e.g. the argon2 PBKDF, which is intentionally memory hungry, hits OOM), we roll + * back the slots added so far and return with the pre-existing slots untouched. The reverse order + * (destroy a slot first, then add its replacement at the same index) would, on such a failure, leave + * the slot destroyed with no replacement, permanently locking the user out. */ - if (i >= n_effective) { - if (r >= 0) - log_info("Destroyed LUKS key slot %zu.", i); - continue; - } + _cleanup_free_ int *new_slots = new(int, n_effective); + if (!new_slots) + return log_oom(); + size_t n_added = 0; + for (size_t i = 0; i < n_effective; i++) { if (password_cache_contains(cache, effective_passwords[i])) { /* Is this a FIDO2 or PKCS#11 password? */ - log_debug("Using minimal PBKDF for slot %zu", i); + log_debug("Using minimal PBKDF for new key slot."); r = sym_crypt_set_pbkdf_type(setup->crypt_device, &minimal_pbkdf); } else { - log_debug("Using good PBKDF for slot %zu", i); + log_debug("Using good PBKDF for new key slot."); r = sym_crypt_set_pbkdf_type(setup->crypt_device, &good_pbkdf); } - if (r < 0) - return log_error_errno(r, "Failed to tweak PBKDF for slot %zu: %m", i); + if (r < 0) { + log_error_errno(r, "Failed to tweak PBKDF for new key slot: %m"); + goto rollback; + } r = sym_crypt_keyslot_add_by_volume_key( setup->crypt_device, - i, + CRYPT_ANY_SLOT, volume_key, volume_key_size, effective_passwords[i], strlen(effective_passwords[i])); - if (r < 0) - return log_error_errno(r, "Failed to set up LUKS password: %m"); + if (r < 0) { + log_error_errno(r, "Failed to set up LUKS password: %m"); + goto rollback; + } - log_info("Updated LUKS key slot %zu.", i); + new_slots[n_added++] = r; + log_info("Added LUKS key slot %i.", r); + } + + /* All replacement slots are committed now; drop every slot that isn't one we just added. */ + for (size_t i = 0; i < max_key_slots; i++) { + bool keep = false; + + for (size_t j = 0; j < n_added; j++) + if (new_slots[j] == (int) i) { + keep = true; + break; + } + if (keep) + continue; + + r = sym_crypt_keyslot_destroy(setup->crypt_device, i); + if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */ + return log_error_errno(r, "Failed to destroy LUKS password: %m"); + if (r >= 0) + log_info("Destroyed LUKS key slot %zu.", i); } return 1; + +rollback: + /* Undo the slots added in this call, so that the header is left exactly as we found it. */ + for (size_t i = 0; i < n_added; i++) + (void) sym_crypt_keyslot_destroy(setup->crypt_device, new_slots[i]); + + return r; } int home_lock_luks(UserRecord *h, HomeSetup *setup) { diff --git a/src/import/pull-oci.c b/src/import/pull-oci.c index c2d9bfeddf..5df55e4278 100644 --- a/src/import/pull-oci.c +++ b/src/import/pull-oci.c @@ -1293,7 +1293,7 @@ static void oci_pull_job_on_finished_layer(PullJob *j) { goto finish; } - assert(set_remove(i->active_layer_jobs, j) == j); + assert_se(set_remove(i->active_layer_jobs, j) == j); r = oci_pull_work(i); if (r <= 0) diff --git a/src/shared/copy.c b/src/shared/copy.c index 21add1d5c0..527022dcca 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -959,7 +959,7 @@ static int fd_copy_fifo( AT_SYMLINK_NOFOLLOW) < 0) r = -errno; - if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0) + if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0) r = -errno; (void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW); @@ -1012,7 +1012,7 @@ static int fd_copy_node( AT_SYMLINK_NOFOLLOW) < 0) r = -errno; - if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0) + if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0) r = -errno; (void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index f9556ea6b4..af19a48eea 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1309,15 +1309,17 @@ static int dissect_image( start = sym_blkid_partition_get_start(pp); if (start < 0) return errno_or_else(EIO); - - assert((uint64_t) start < UINT64_MAX/512); + if ((uint64_t) start >= UINT64_MAX/512) + return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW), + "Partition start LBA too large to convert to a byte offset, refusing."); errno = 0; size = sym_blkid_partition_get_size(pp); if (size < 0) return errno_or_else(EIO); - - assert((uint64_t) size < UINT64_MAX/512); + if ((uint64_t) size >= UINT64_MAX/512) + return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW), + "Partition size in LBA too large to convert to a byte offset, refusing."); /* While probing we need the non-diskseq device node name to access the thing, hence mask off * DISSECT_IMAGE_DISKSEQ_DEVNODE. */ diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 290a6bed8f..8465d6cc99 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -308,6 +309,69 @@ TEST(copy_tree_at_symlink) { fd = safe_close(fd); } +TEST(copy_tree_fifo_chmod_symlink_race) { + _cleanup_(rm_rf_physical_and_freep) char *srcp = NULL, *dstp = NULL; + _cleanup_close_ int src = -EBADF, dst = -EBADF; + _cleanup_close_pair_ int ready[2] = EBADF_PAIR, stop[2] = EBADF_PAIR; + struct stat st; + int status; + pid_t pid; + char c; + + if (!slow_tests_enabled()) + return (void) log_tests_skipped("slow tests are disabled"); + + ASSERT_OK(src = mkdtemp_open(NULL, 0, &srcp)); + ASSERT_OK(dst = mkdtemp_open(NULL, 0, &dstp)); + ASSERT_OK_ERRNO(mkfifoat(src, "fifo", 0777)); + ASSERT_OK_ERRNO(fchmodat(src, "fifo", 0777, 0)); + ASSERT_OK(write_string_file_at(dst, "victim", "victim", WRITE_STRING_FILE_CREATE)); + ASSERT_OK_ERRNO(fchmodat(dst, "victim", 0600, 0)); + + ASSERT_OK_ERRNO(pipe2(ready, O_CLOEXEC)); + ASSERT_OK_ERRNO(pipe2(stop, O_CLOEXEC|O_NONBLOCK)); + + pid = ASSERT_OK_ERRNO(fork()); + if (pid == 0) { + ready[0] = safe_close(ready[0]); + stop[1] = safe_close(stop[1]); + + assert_se(write(ready[1], &(const char) { 'x' }, 1) == 1); + + for (;;) { + char stop_char; + + ssize_t n = read(stop[0], &stop_char, 1); + if (n >= 0) + _exit(EXIT_SUCCESS); + + (void) unlinkat(dst, "fifo", 0); + (void) symlinkat("victim", dst, "fifo"); + } + } + + ready[1] = safe_close(ready[1]); + stop[0] = safe_close(stop[0]); + + ASSERT_OK_EQ_ERRNO(read(ready[0], &c, 1), 1); + + bool changed = false; + for (unsigned i = 0; i < 20000; i++) { + (void) copy_tree_at(src, "fifo", dst, "fifo", UID_INVALID, GID_INVALID, COPY_REPLACE, NULL, NULL); + + ASSERT_OK_ERRNO(fstatat(dst, "victim", &st, 0)); + if ((st.st_mode & 0777) != 0600) { + changed = true; + break; + } + } + + ASSERT_OK_EQ_ERRNO(write(stop[1], &(const char) { 'x' }, 1), 1); + ASSERT_OK_ERRNO(waitpid(pid, &status, 0)); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_FALSE(changed); +} + TEST_RET(copy_bytes) { _cleanup_close_pair_ int pipefd[2] = EBADF_PAIR; _cleanup_close_ int infd = -EBADF; diff --git a/src/test/test-ellipsize.c b/src/test/test-ellipsize.c index 2efb123ac2..3d47cfebe2 100644 --- a/src/test/test-ellipsize.c +++ b/src/test/test-ellipsize.c @@ -75,6 +75,12 @@ TEST(ellipsize_mem) { test_ellipsize_mem_one(s, l, k); } +TEST(ellipsize_mem_truncated_utf8) { + static const char s[] = { 'a', '\xf0', '\x80', '\x80', '\x80', 'b' }; + + ASSERT_NULL(ellipsize_mem(s, 2, 1, 50)); +} + static void test_ellipsize_one(const char *p) { _cleanup_free_ char *t = NULL; t = ellipsize(p, columns(), 70); diff --git a/src/test/test-log.c b/src/test/test-log.c index fb23776b21..61468a3229 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -145,6 +145,28 @@ TEST(log_format_iovec) { LOG_ITEM("FOOFOO=%hu-%llu", (unsigned short) 4, (long long unsigned) 3)); } +TEST(log_struct_iovec_many_fields) { + LogTarget old_target = log_get_target(); + struct iovec iovec[1024]; + + log_set_target(LOG_TARGET_JOURNAL); + log_open(); + if (log_on_console()) { + log_set_target(old_target); + log_open(); + return (void) log_tests_skipped("journal socket is not available"); + } + + iovec[0] = IOVEC_MAKE_STRING("MESSAGE=many fields"); + for (size_t i = 1; i < ELEMENTSOF(iovec); i++) + iovec[i] = IOVEC_MAKE_STRING("FIELD=value"); + + ASSERT_OK(log_struct_iovec(LOG_INFO, iovec, ELEMENTSOF(iovec))); + + log_set_target(old_target); + log_open(); +} + static void test_log_struct(void) { log_struct(LOG_INFO, "MESSAGE=Waldo PID="PID_FMT" (no errno)", getpid_cached(),