Skip to content

Commit aa23668

Browse files
committed
fix: eliminate installer filesystem races
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 50df099 commit aa23668

7 files changed

Lines changed: 474 additions & 124 deletions

File tree

‎src/cli/cli.c‎

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3632,41 +3632,8 @@ static int cbm_powershell_quote_word(const char *value, char *out, size_t out_si
36323632
static bool cbm_write_owned_hook_script_with_legacy(const char *path, const char *script,
36333633
const char *const *legacy_scripts,
36343634
size_t legacy_count) {
3635-
int write_result = cbm_text_ensure_owned_document(path, script);
3636-
for (size_t index = 0U; write_result != 0 && index < legacy_count; index++) {
3637-
const char *legacy = legacy_scripts ? legacy_scripts[index] : NULL;
3638-
if (legacy) {
3639-
write_result =
3640-
cbm_text_write_owned_document_if_unchanged(path, script, legacy, strlen(legacy));
3641-
}
3642-
}
3643-
if (write_result != 0) {
3644-
return false;
3645-
}
3646-
#ifndef _WIN32
3647-
struct stat expected;
3648-
if (lstat(path, &expected) != 0 || !S_ISREG(expected.st_mode) || expected.st_nlink != 1U) {
3649-
return false;
3650-
}
3651-
int flags = O_RDONLY;
3652-
#ifdef O_NOFOLLOW
3653-
flags |= O_NOFOLLOW;
3654-
#endif
3655-
#ifdef O_CLOEXEC
3656-
flags |= O_CLOEXEC;
3657-
#endif
3658-
int descriptor = open(path, flags);
3659-
struct stat state;
3660-
bool ok = descriptor >= 0 && fstat(descriptor, &state) == 0 && S_ISREG(state.st_mode) &&
3661-
state.st_nlink == 1U && state.st_dev == expected.st_dev &&
3662-
state.st_ino == expected.st_ino && fchmod(descriptor, CLI_OCTAL_PERM) == 0;
3663-
if (descriptor >= 0) {
3664-
(void)close(descriptor);
3665-
}
3666-
return ok;
3667-
#else
3668-
return chmod(path, CLI_OCTAL_PERM) == 0;
3669-
#endif
3635+
return cbm_text_migrate_owned_document_mode(path, script, legacy_scripts, legacy_count,
3636+
CLI_OCTAL_PERM) == CLI_OK;
36703637
}
36713638

36723639
static bool cbm_write_owned_hook_script(const char *path, const char *script) {

‎src/cli/config_text_edit.c‎

Lines changed: 124 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,15 @@
4343
#define TEXT_MAX_MARKER_BYTES 4096U
4444
#define TEXT_TEMP_SUFFIX_BYTES 80U
4545
#define TEXT_TEMP_ATTEMPTS 64U
46+
#define TEXT_OWNER_READ 0400U
4647

4748
#ifdef CBM_TEXT_EDIT_ENABLE_TEST_API
4849
static CBM_TLS cbm_text_precommit_test_hook_t text_precommit_test_hook = NULL;
4950
static CBM_TLS void *text_precommit_test_context = NULL;
5051
static CBM_TLS cbm_text_precommit_test_hook_t text_prepublish_test_hook = NULL;
5152
static CBM_TLS void *text_prepublish_test_context = NULL;
53+
static CBM_TLS cbm_text_precommit_test_hook_t text_temp_closed_test_hook = NULL;
54+
static CBM_TLS void *text_temp_closed_test_context = NULL;
5255
#endif
5356

5457
static atomic_uint text_temp_sequence = 1U;
@@ -287,6 +290,20 @@ static int text_snapshot_equal(const text_file_snapshot_t *left,
287290
#endif
288291
}
289292

293+
static int text_snapshot_publication_equal(const text_file_snapshot_t *trusted,
294+
const text_file_snapshot_t *reopened) {
295+
#ifdef _WIN32
296+
return trusted->exists == reopened->exists && trusted->exists &&
297+
trusted->volume_serial == reopened->volume_serial &&
298+
trusted->file_index_high == reopened->file_index_high &&
299+
trusted->file_index_low == reopened->file_index_low &&
300+
trusted->attributes == reopened->attributes &&
301+
trusted->link_count == reopened->link_count && trusted->size == reopened->size;
302+
#else
303+
return text_snapshot_equal(trusted, reopened);
304+
#endif
305+
}
306+
290307
#ifdef _WIN32
291308
static int text_snapshot_from_handle(HANDLE handle, text_file_snapshot_t *snapshot) {
292309
BY_HANDLE_FILE_INFORMATION info;
@@ -344,6 +361,27 @@ static int text_snapshot_from_stat(const struct stat *state, text_file_snapshot_
344361
}
345362
#endif
346363

364+
static int text_snapshot_from_file(FILE *file, text_file_snapshot_t *snapshot) {
365+
if (!file || !snapshot) {
366+
return TEXT_ERROR;
367+
}
368+
#ifdef _WIN32
369+
intptr_t native_handle = _get_osfhandle(cbm_fileno(file));
370+
return native_handle == -1
371+
? TEXT_ERROR
372+
: text_snapshot_from_handle((HANDLE)(uintptr_t)native_handle, snapshot);
373+
#else
374+
struct stat state;
375+
return fstat(cbm_fileno(file), &state) == 0 ? text_snapshot_from_stat(&state, snapshot)
376+
: TEXT_ERROR;
377+
#endif
378+
}
379+
380+
static int text_requested_mode_valid(int override_mode, unsigned int requested_mode) {
381+
return !override_mode ||
382+
((requested_mode & ~0777U) == 0U && (requested_mode & TEXT_OWNER_READ) != 0U);
383+
}
384+
347385
static int text_read_file(const char *path, char **data_out, size_t *len_out,
348386
text_file_snapshot_t *snapshot_out) {
349387
if (!path || !data_out || !len_out || !snapshot_out) {
@@ -551,13 +589,23 @@ static int text_replace_file(const char *temp_path, const char *path, int destin
551589
#endif
552590
}
553591

554-
static int text_write_atomic(const char *path, const char *new_data, size_t new_len,
555-
const char *old_data, size_t old_len,
556-
const text_file_snapshot_t *snapshot) {
557-
if (new_len > TEXT_MAX_BYTES || old_len > TEXT_MAX_BYTES) {
592+
static int text_write_atomic_mode(const char *path, const char *new_data, size_t new_len,
593+
const char *old_data, size_t old_len,
594+
const text_file_snapshot_t *snapshot, int override_mode,
595+
unsigned int requested_mode) {
596+
if (new_len > TEXT_MAX_BYTES || old_len > TEXT_MAX_BYTES ||
597+
!text_requested_mode_valid(override_mode, requested_mode)) {
558598
return TEXT_ERROR;
559599
}
560-
if (new_len == old_len && (new_len == 0U || memcmp(new_data, old_data, new_len) == 0)) {
600+
int content_same =
601+
new_len == old_len && (new_len == 0U || memcmp(new_data, old_data, new_len) == 0);
602+
#ifndef _WIN32
603+
int mode_same =
604+
snapshot->exists && (snapshot->mode & 0777U) == (mode_t)(requested_mode & 0777U);
605+
#else
606+
int mode_same = snapshot->exists;
607+
#endif
608+
if (content_same && (!override_mode || mode_same)) {
561609
return TEXT_OK;
562610
}
563611
size_t path_len = 0U;
@@ -583,7 +631,9 @@ static int text_write_atomic(const char *path, const char *new_data, size_t new_
583631
}
584632
errno = 0;
585633
#ifdef _WIN32
586-
file = cbm_fopen(temp_path, "wbx");
634+
/* The descriptor-bound identity snapshot uses GetFileInformationByHandle,
635+
* so request read access as well as exclusive binary creation. */
636+
file = cbm_fopen(temp_path, "w+bx");
587637
#else
588638
#ifndef O_NOFOLLOW
589639
free(temp_path);
@@ -627,14 +677,27 @@ static int text_write_atomic(const char *path, const char *new_data, size_t new_
627677
fchown(cbm_fileno(file), snapshot->owner, snapshot->group) != 0) {
628678
failed = 1;
629679
}
630-
mode_t mode = snapshot->exists ? snapshot->mode & 0777U : 0600U;
680+
mode_t mode = override_mode ? (mode_t)(requested_mode & 0777U)
681+
: snapshot->exists ? snapshot->mode & 0777U
682+
: 0600U;
631683
if (!failed && fchmod(cbm_fileno(file), mode) != 0) {
632684
failed = 1;
633685
}
634686
#endif
635687
if (!failed && TEXT_SYNC(cbm_fileno(file)) != 0) {
636688
failed = 1;
637689
}
690+
text_file_snapshot_t trusted_temp_snapshot = {0};
691+
if (!failed && text_snapshot_from_file(file, &trusted_temp_snapshot) != TEXT_OK) {
692+
failed = 1;
693+
}
694+
#ifndef _WIN32
695+
if (!failed && override_mode &&
696+
(trusted_temp_snapshot.owner != geteuid() ||
697+
(trusted_temp_snapshot.mode & 0777U) != (mode_t)(requested_mode & 0777U))) {
698+
failed = 1;
699+
}
700+
#endif
638701
if (fclose(file) != 0) {
639702
failed = 1;
640703
}
@@ -643,12 +706,17 @@ static int text_write_atomic(const char *path, const char *new_data, size_t new_
643706
free(temp_path);
644707
return TEXT_ERROR;
645708
}
709+
#ifdef CBM_TEXT_EDIT_ENABLE_TEST_API
710+
if (text_temp_closed_test_hook) {
711+
text_temp_closed_test_hook(temp_path, text_temp_closed_test_context);
712+
}
713+
#endif
646714
char *temp_data = NULL;
647715
size_t temp_len = 0U;
648716
text_file_snapshot_t temp_snapshot;
649717
if (text_read_file(temp_path, &temp_data, &temp_len, &temp_snapshot) != TEXT_OK ||
650-
!temp_snapshot.exists || temp_len != new_len ||
651-
(new_len != 0U && memcmp(temp_data, new_data, new_len) != 0)) {
718+
!text_snapshot_publication_equal(&trusted_temp_snapshot, &temp_snapshot) ||
719+
temp_len != new_len || (new_len != 0U && memcmp(temp_data, new_data, new_len) != 0)) {
652720
free(temp_data);
653721
(void)cbm_unlink(temp_path);
654722
free(temp_path);
@@ -682,6 +750,12 @@ static int text_write_atomic(const char *path, const char *new_data, size_t new_
682750
return TEXT_OK;
683751
}
684752

753+
static int text_write_atomic(const char *path, const char *new_data, size_t new_len,
754+
const char *old_data, size_t old_len,
755+
const text_file_snapshot_t *snapshot) {
756+
return text_write_atomic_mode(path, new_data, new_len, old_data, old_len, snapshot, 0, 0U);
757+
}
758+
685759
static int text_delete_file(const char *path, const char *old_data, size_t old_len,
686760
const text_file_snapshot_t *snapshot) {
687761
#ifdef CBM_TEXT_EDIT_ENABLE_TEST_API
@@ -726,6 +800,11 @@ void cbm_text_set_prepublish_hook_for_testing(cbm_text_precommit_test_hook_t hoo
726800
text_prepublish_test_hook = hook;
727801
text_prepublish_test_context = context;
728802
}
803+
804+
void cbm_text_set_temp_closed_hook_for_testing(cbm_text_precommit_test_hook_t hook, void *context) {
805+
text_temp_closed_test_hook = hook;
806+
text_temp_closed_test_context = context;
807+
}
729808
#endif
730809

731810
static int text_bytes_equal(const char *data, size_t start, size_t end, const char *value,
@@ -1143,13 +1222,15 @@ static int text_matches_candidate(const char *data, size_t data_len, const char
11431222
return TEXT_OK;
11441223
}
11451224

1146-
int cbm_text_migrate_owned_document(const char *file_path, const char *current_content,
1147-
const char *const *released_contents, size_t released_count) {
1225+
static int text_migrate_owned_document(const char *file_path, const char *current_content,
1226+
const char *const *released_contents, size_t released_count,
1227+
int override_mode, unsigned int requested_mode) {
11481228
size_t current_len = 0U;
11491229
if (!text_valid_path(file_path) ||
11501230
text_bounded_strlen(current_content, TEXT_MAX_BYTES, &current_len) != TEXT_OK ||
11511231
text_validate_bytes(current_content, current_len, 1) != TEXT_OK ||
1152-
(released_count > 0U && !released_contents)) {
1232+
(released_count > 0U && !released_contents) ||
1233+
!text_requested_mode_valid(override_mode, requested_mode)) {
11531234
return TEXT_ERROR;
11541235
}
11551236
for (size_t i = 0U; i < released_count; i++) {
@@ -1167,9 +1248,15 @@ int cbm_text_migrate_owned_document(const char *file_path, const char *current_c
11671248
free(old_data);
11681249
return TEXT_ERROR;
11691250
}
1251+
#ifndef _WIN32
1252+
if (override_mode && snapshot.exists && snapshot.owner != geteuid()) {
1253+
free(old_data);
1254+
return TEXT_ERROR;
1255+
}
1256+
#endif
11701257
if (!snapshot.exists) {
1171-
int result = text_write_atomic(file_path, current_content, current_len, old_data, old_len,
1172-
&snapshot);
1258+
int result = text_write_atomic_mode(file_path, current_content, current_len, old_data,
1259+
old_len, &snapshot, override_mode, requested_mode);
11731260
free(old_data);
11741261
return result;
11751262
}
@@ -1180,6 +1267,14 @@ int cbm_text_migrate_owned_document(const char *file_path, const char *current_c
11801267
return TEXT_ERROR;
11811268
}
11821269
if (matches) {
1270+
#ifndef _WIN32
1271+
if (override_mode && (snapshot.mode & 0777U) != (mode_t)(requested_mode & 0777U)) {
1272+
int result = text_write_atomic_mode(file_path, current_content, current_len, old_data,
1273+
old_len, &snapshot, override_mode, requested_mode);
1274+
free(old_data);
1275+
return result;
1276+
}
1277+
#endif
11831278
free(old_data);
11841279
return TEXT_OK;
11851280
}
@@ -1189,8 +1284,8 @@ int cbm_text_migrate_owned_document(const char *file_path, const char *current_c
11891284
return TEXT_ERROR;
11901285
}
11911286
if (matches) {
1192-
int result = text_write_atomic(file_path, current_content, current_len, old_data,
1193-
old_len, &snapshot);
1287+
int result = text_write_atomic_mode(file_path, current_content, current_len, old_data,
1288+
old_len, &snapshot, override_mode, requested_mode);
11941289
free(old_data);
11951290
return result;
11961291
}
@@ -1199,6 +1294,19 @@ int cbm_text_migrate_owned_document(const char *file_path, const char *current_c
11991294
return TEXT_UNOWNED;
12001295
}
12011296

1297+
int cbm_text_migrate_owned_document(const char *file_path, const char *current_content,
1298+
const char *const *released_contents, size_t released_count) {
1299+
return text_migrate_owned_document(file_path, current_content, released_contents,
1300+
released_count, 0, 0U);
1301+
}
1302+
1303+
int cbm_text_migrate_owned_document_mode(const char *file_path, const char *current_content,
1304+
const char *const *released_contents,
1305+
size_t released_count, unsigned int mode) {
1306+
return text_migrate_owned_document(file_path, current_content, released_contents,
1307+
released_count, 1, mode);
1308+
}
1309+
12021310
int cbm_text_remove_owned_document(const char *file_path, const char *expected_owned_content) {
12031311
size_t expected_len = 0U;
12041312
if (!text_valid_path(file_path) ||

‎src/cli/config_text_edit.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ int cbm_text_ensure_owned_document(const char *file_path, const char *owned_cont
3131
* 1 for user-modified/unowned content. */
3232
int cbm_text_migrate_owned_document(const char *file_path, const char *current_content,
3333
const char *const *released_contents, size_t released_count);
34+
/* As above, but publish the owned document with the requested low permission
35+
* bits as part of the same atomic replacement. The mode must include owner-read
36+
* permission. Existing POSIX documents must be owned by the effective user.
37+
* On Windows the mode is validated but otherwise ignored because command
38+
* scripts do not use POSIX execute bits. */
39+
int cbm_text_migrate_owned_document_mode(const char *file_path, const char *current_content,
40+
const char *const *released_contents,
41+
size_t released_count, unsigned int mode);
3442
/* Returns 0 when removed/missing, 1 when a regular document exists but is not
3543
* byte-for-byte owned by the caller, and -1 for unsafe state or I/O failure. */
3644
int cbm_text_remove_owned_document(const char *file_path, const char *expected_owned_content);
@@ -44,6 +52,9 @@ void cbm_text_set_precommit_hook_for_testing(cbm_text_precommit_test_hook_t hook
4452
/* Runs after the first stale-snapshot check and before the final identity
4553
* revalidation used for publication or exact deletion. */
4654
void cbm_text_set_prepublish_hook_for_testing(cbm_text_precommit_test_hook_t hook, void *context);
55+
/* Runs after the synced temporary descriptor is closed but before its pathname
56+
* is reopened and matched against the descriptor-bound snapshot. */
57+
void cbm_text_set_temp_closed_hook_for_testing(cbm_text_precommit_test_hook_t hook, void *context);
4758
#endif
4859

4960
#endif

0 commit comments

Comments
 (0)