From 8965edb843d441a6671732e5e09a7066315aba63 Mon Sep 17 00:00:00 2001 From: bluezr Date: Thu, 6 Aug 2026 14:21:23 -0700 Subject: [PATCH 1/7] wallet, seal: create private files 0600 instead of at the umask The wallet database and the sealed seed files were created with plain fopen(), so their permissions came from the process umask. Under the common 0002 that is 0664: readable by every local user on the machine, and writable by the group. plain fopen() : 0664 dogecoin_fopen_private() : 0600 What each file holds decides how much that matters. The wallet database carries WALLET_DB_REC_TYPE_MASTERPUBKEY along with the address and transaction records. No private keys -- those live in the sealed files -- but a master public key is enough to derive every address the wallet will ever use and reconstruct its whole transaction history. World-readable is the wrong default for that. The seal files hold encrypted seeds and mnemonics. Encryption means a leak is not an immediate compromise, but the encryption is derived from a password, and handing a local attacker the ciphertext is an invitation to work on it offline at their leisure. Adds dogecoin_fopen_private(), which opens with O_CREAT and 0600 on POSIX and defers to fopen() on Windows, where there is no umask and a new file inherits the directory ACL. Four call sites move over: one in dogecoin_wallet_create and three in seal.c. The mode applies only when the file is created, so an existing wallet or seal file keeps whatever permissions it already has. This tightens new files without silently changing anyone's current ones -- and a note for whoever picks that up, since it does mean existing installs stay as they are until the file is recreated. The test asserts the created mode is exactly 0600, that no group or other bits are set at all, and that reopening an existing file does not widen it. Skipped on Windows, where the mode has no meaning. 82/82. --- include/dogecoin/utils.h | 7 ++++++ src/seal.c | 6 ++--- src/utils.c | 40 ++++++++++++++++++++++++++++++++ src/wallet.c | 2 +- test/unittester.c | 2 ++ test/utils_tests.c | 50 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 103 insertions(+), 4 deletions(-) diff --git a/include/dogecoin/utils.h b/include/dogecoin/utils.h index da2d0fdf7..a08ac1a80 100644 --- a/include/dogecoin/utils.h +++ b/include/dogecoin/utils.h @@ -83,6 +83,13 @@ LIBDOGECOIN_API void print_bits(size_t const size, void const* ptr); LIBDOGECOIN_API void prepend(char* s, const char* t); LIBDOGECOIN_API void append(char* s, char* t); LIBDOGECOIN_API char* concat(char* prefix, char* suffix); +/* Open a file that should not be readable by other local users, creating it + 0600 where the platform has POSIX permissions. Use for anything holding key + material, encrypted or not, and for the wallet database -- it carries the + master public key, which is enough to derive every address and reconstruct + the transaction history. An existing file keeps its current mode. */ +LIBDOGECOIN_API FILE* dogecoin_fopen_private(const char* path, const char* mode); + LIBDOGECOIN_API void slice(const char *str, char *result, size_t start, size_t end); LIBDOGECOIN_API void replace_last_after_delim(const char *str, char* delim, char* replacement); LIBDOGECOIN_API void text_to_hex(char* in, char* out); diff --git a/src/seal.c b/src/seal.c index 398084256..f988de5d8 100644 --- a/src/seal.c +++ b/src/seal.c @@ -1256,7 +1256,7 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_encrypt_seed_with_sw(const SEED seed, con fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = fopen(fullpath, overwrite ? "wb+" : "wb"); + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { @@ -2113,7 +2113,7 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_generate_hdnode_encrypt_with_sw(dogecoin_ fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = fopen(fullpath, overwrite ? "wb+" : "wb"); + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { @@ -2996,7 +2996,7 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_generate_mnemonic_encrypt_with_sw(MNEMONI fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = fopen(fullpath, overwrite ? "wb+" : "wb"); + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { diff --git a/src/utils.c b/src/utils.c index f0af3f94d..df3b64098 100644 --- a/src/utils.c +++ b/src/utils.c @@ -32,6 +32,11 @@ #endif #include +#ifndef _WIN32 +#include +#include +#include +#endif #include #include #include @@ -678,6 +683,41 @@ char* concat(char* prefix, char* suffix) { return file; } + +FILE* dogecoin_fopen_private(const char* path, const char* mode) +{ +#ifdef _WIN32 + /* No umask on Windows; a new file inherits the directory's ACL, so plain + fopen already gets whatever the parent grants. */ + return fopen(path, mode); +#else + int flags = O_RDWR | O_CREAT; + int fd; + FILE* fp; + + if (!path || !mode) { + return NULL; + } + if (strchr(mode, 'a')) { + flags |= O_APPEND; + } else if (strchr(mode, 'w')) { + flags |= O_TRUNC; + } + /* 0600. The mode only applies when the file is created, so an existing + file keeps whatever permissions it already had -- this tightens new + files without silently changing anyone's existing ones. */ + fd = open(path, flags, S_IRUSR | S_IWUSR); + if (fd < 0) { + return NULL; + } + fp = fdopen(fd, mode); + if (!fp) { + close(fd); + } + return fp; +#endif +} + void slice(const char *str, char *result, size_t start, size_t end) { strncpy(result, str + start, end - start); diff --git a/src/wallet.c b/src/wallet.c index 6c567d3f2..55f2be578 100644 --- a/src/wallet.c +++ b/src/wallet.c @@ -980,7 +980,7 @@ dogecoin_bool dogecoin_wallet_create(dogecoin_wallet* wallet, const char* file_p // open wallet file if not already open if (!wallet->dbfile) { - wallet->dbfile = fopen(file_path, "a+b"); + wallet->dbfile = dogecoin_fopen_private(file_path, "a+b"); if (wallet->dbfile) { snprintf((char*)wallet->filename, sizeof(wallet->filename), "%s", file_path); } diff --git a/test/unittester.c b/test/unittester.c index df5181155..e28849017 100644 --- a/test/unittester.c +++ b/test/unittester.c @@ -99,6 +99,7 @@ extern void test_invalid_tx_deser(); extern void test_tx_sign(); extern void test_scripts(); extern void test_utils(); +extern void test_utils_fopen_private(); extern void test_vector(); extern void test_qr(); @@ -236,6 +237,7 @@ int main() u_run_test(test_script_parse); u_run_test(test_script_op_codeseperator); u_run_test(test_utils); + u_run_test(test_utils_fopen_private); u_run_test(test_vector); u_run_test(test_qr); diff --git a/test/utils_tests.c b/test/utils_tests.c index af2fc90ea..dd382bac3 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -8,6 +8,10 @@ #include #include +#ifndef _WIN32 +#include +#include +#endif #include /* test a buffer overflow protection */ @@ -132,3 +136,49 @@ void test_dit() debug_print("%s", "DIT test: disabled (DIT not supported)\n"); } } + + +/* + * The wallet database and the sealed seed files were created with plain + * fopen(), so their mode came from the process umask -- 0664 under the common + * 0002, i.e. readable by every local user and writable by the group. + * + * The wallet database holds the master public key, which is enough to derive + * every address and reconstruct the wallet's transaction history. The seal + * files hold encrypted seeds and mnemonics; encryption means a leak is not an + * immediate compromise, but handing out the ciphertext invites an offline + * attack on a password-derived key. + */ +void test_utils_fopen_private() +{ +#ifndef _WIN32 + const char* path = "dogecoin_fopen_private_test.tmp"; + struct stat st; + FILE* fp; + + remove(path); + + fp = dogecoin_fopen_private(path, "wb"); + u_assert_true(fp != NULL); + fputc('x', fp); + fclose(fp); + + u_assert_int_eq(stat(path, &st), 0); + /* Owner read/write only: no group or other bits at all. */ + u_assert_int_eq((int)(st.st_mode & 07777), 0600); + u_assert_int_eq((int)(st.st_mode & (S_IRWXG | S_IRWXO)), 0); + + /* Reopening must not widen the mode of a file that already exists. */ + fp = dogecoin_fopen_private(path, "a+b"); + u_assert_true(fp != NULL); + fclose(fp); + u_assert_int_eq(stat(path, &st), 0); + u_assert_int_eq((int)(st.st_mode & 07777), 0600); + + remove(path); + + /* NULL arguments are refused rather than passed through. */ + u_assert_true(dogecoin_fopen_private(NULL, "wb") == NULL); + u_assert_true(dogecoin_fopen_private(path, NULL) == NULL); +#endif +} From 4b6150248b74e8614703399865dce58510e15742 Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 16:46:02 -0700 Subject: [PATCH 2/7] wallet, seal, utils: create private files on the path that actually creates them The private open added for the wallet database sat in dogecoin_wallet_create(), which opens the file only when wallet->dbfile is still NULL. By the time it is called, dogecoin_wallet_load() has already opened the file with plain fopen and set that field, so the guarded branch never ran on the creation path and the wallet database was still created at the process umask -- world-readable under a permissive one. Moved to the line that creates it. dogecoin_fopen_private() also set O_RDWR | O_CREAT for every mode, so "r" created a file that did not exist rather than failing, and a read-only file could not be opened at all. That matters most here: "open the existing wallet" and "start a new one" are different operations, and conflating them can present an empty wallet as a real one. The stdio mode is now translated properly, with C11 'x' mapping to O_EXCL. seal.c decided whether to create by calling access() and then opening, which is a race anyone able to write the directory wins by planting a symlink between the two calls -- and the file at the end of it holds an encrypted seed. All three POSIX sites now create exclusively and report the collision from EEXIST, so the check and the create are one operation. The directory those live in was created with mkdir(CRYPTO_DIR_PATH, 0777) at all three sites, which is what made the race worth running. Now 0700. Tests: a wallet created under umask 0022 must land 0600 -- it does not with the plain fopen restored, which is the point -- and dogecoin_fopen_private must refuse to create for "r", create privately for "w", and refuse an existing file for "wx". 83/83, clean under ASan and UBSan. --- src/seal.c | 30 +++++++++++++++++++++--------- src/utils.c | 32 +++++++++++++++++++++++++++----- src/wallet.c | 8 +++++++- test/unittester.c | 2 ++ test/utils_tests.c | 26 ++++++++++++++++++++++++++ test/wallet_tests.c | 34 ++++++++++++++++++++++++++++++++++ 6 files changed, 117 insertions(+), 15 deletions(-) diff --git a/src/seal.c b/src/seal.c index f988de5d8..7776d6331 100644 --- a/src/seal.c +++ b/src/seal.c @@ -1244,19 +1244,23 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_encrypt_seed_with_sw(const SEED seed, con } fp = _wfopen(fullpath, overwrite ? L"wb+" : L"wb"); #else - if (mkdir(CRYPTO_DIR_PATH, 0777) == -1 && errno != EEXIST) + if (mkdir(CRYPTO_DIR_PATH, 0700) == -1 && errno != EEXIST) { fprintf(stderr, "ERROR: Failed to create directory\n"); return false; } char fullpath[FILE_PATH_MAX_LEN] = {0}; snprintf(fullpath, sizeof(fullpath), SEED_SW_FILE_NAME, file_num); - if (!overwrite && access(fullpath, F_OK) != -1) + /* Exclusive create rather than access() then open(): between those + two calls anyone able to write the directory can plant a symlink + and redirect an encrypted seed somewhere of their choosing. + O_EXCL both reports the collision and refuses to follow one. */ + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wbx"); + if (!fp && !overwrite && errno == EEXIST) { fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { @@ -2101,19 +2105,23 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_generate_hdnode_encrypt_with_sw(dogecoin_ } fp = _wfopen(fullpath, overwrite ? L"wb+" : L"wb"); #else - if (mkdir(CRYPTO_DIR_PATH, 0777) == -1 && errno != EEXIST) + if (mkdir(CRYPTO_DIR_PATH, 0700) == -1 && errno != EEXIST) { fprintf(stderr, "ERROR: Failed to create directory\n"); return false; } char fullpath[FILE_PATH_MAX_LEN] = {0}; snprintf(fullpath, sizeof(fullpath), MASTER_SW_FILE_NAME, file_num); - if (!overwrite && access(fullpath, F_OK) != -1) + /* Exclusive create rather than access() then open(): between those + two calls anyone able to write the directory can plant a symlink + and redirect an encrypted seed somewhere of their choosing. + O_EXCL both reports the collision and refuses to follow one. */ + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wbx"); + if (!fp && !overwrite && errno == EEXIST) { fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { @@ -2984,19 +2992,23 @@ LIBDOGECOIN_API dogecoin_bool dogecoin_generate_mnemonic_encrypt_with_sw(MNEMONI } fp = _wfopen(fullpath, overwrite ? L"wb+" : L"wb"); #else - if (mkdir(CRYPTO_DIR_PATH, 0777) == -1 && errno != EEXIST) + if (mkdir(CRYPTO_DIR_PATH, 0700) == -1 && errno != EEXIST) { fprintf(stderr, "ERROR: Failed to create directory\n"); return false; } char fullpath[FILE_PATH_MAX_LEN] = {0}; snprintf(fullpath, sizeof(fullpath), MNEMONIC_SW_FILE_NAME, file_num); - if (!overwrite && access(fullpath, F_OK) != -1) + /* Exclusive create rather than access() then open(): between those + two calls anyone able to write the directory can plant a symlink + and redirect an encrypted seed somewhere of their choosing. + O_EXCL both reports the collision and refuses to follow one. */ + fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wbx"); + if (!fp && !overwrite && errno == EEXIST) { fprintf(stderr, "ERROR: File already exists. Use overwrite flag to replace it.\n"); return false; } - fp = dogecoin_fopen_private(fullpath, overwrite ? "wb+" : "wb"); #endif if (!fp) { diff --git a/src/utils.c b/src/utils.c index df3b64098..74f4f2413 100644 --- a/src/utils.c +++ b/src/utils.c @@ -691,17 +691,39 @@ FILE* dogecoin_fopen_private(const char* path, const char* mode) fopen already gets whatever the parent grants. */ return fopen(path, mode); #else - int flags = O_RDWR | O_CREAT; + int flags; int fd; FILE* fp; if (!path || !mode) { return NULL; } - if (strchr(mode, 'a')) { - flags |= O_APPEND; - } else if (strchr(mode, 'w')) { - flags |= O_TRUNC; + + /* Translate the stdio mode rather than assuming it creates. + * + * O_CREAT was previously unconditional, so "r"/"r+" -- open an existing + * file -- would create one instead of failing, and a read-only file could + * not be opened at all because O_RDWR was also unconditional. Both matter + * here: this is the wallet path, where "open the existing wallet" and + * "start a new one" are different operations and conflating them can + * present an empty wallet as a real one. */ + switch (mode[0]) { + case 'r': + flags = strchr(mode, '+') ? O_RDWR : O_RDONLY; + break; + case 'w': + flags = (strchr(mode, '+') ? O_RDWR : O_WRONLY) | O_CREAT; + /* C11 'x': create exclusively, fail if the path exists. Callers use + it to replace a check-then-open, which is a race an attacker with + write access to the directory wins by planting a symlink between + the two calls. O_EXCL also refuses to follow one. */ + flags |= strchr(mode, 'x') ? O_EXCL : O_TRUNC; + break; + case 'a': + flags = (strchr(mode, '+') ? O_RDWR : O_WRONLY) | O_CREAT | O_APPEND; + break; + default: + return NULL; } /* 0600. The mode only applies when the file is created, so an existing file keeps whatever permissions it already had -- this tightens new diff --git a/src/wallet.c b/src/wallet.c index 55f2be578..8961da440 100644 --- a/src/wallet.c +++ b/src/wallet.c @@ -1146,7 +1146,13 @@ dogecoin_bool dogecoin_wallet_load(dogecoin_wallet* wallet, const char* file_pat } } - wallet->dbfile = fopen(file_path, *created ? "a+b" : "r+b"); + /* The creation path runs here, not in dogecoin_wallet_create(): that + function opens the file only when wallet->dbfile is still NULL, and by + the time it is called below this line has already set it. Opening + privately there tightened nothing, so the wallet database was created at + the process umask -- world-readable under a permissive one. */ + wallet->dbfile = *created ? dogecoin_fopen_private(file_path, "a+b") + : fopen(file_path, "r+b"); if (wallet->dbfile) { snprintf((char*)wallet->filename, sizeof(wallet->filename), "%s", file_path); } diff --git a/test/unittester.c b/test/unittester.c index e28849017..3cbcf2b1d 100644 --- a/test/unittester.c +++ b/test/unittester.c @@ -134,6 +134,7 @@ extern void test_examples(); #ifdef WITH_WALLET extern void test_wallet_basics(); extern void test_wallet(); +extern void test_wallet_file_is_private(); extern void test_wallet_malformed_reclen(); extern void test_wallet_reorg_utxo_update(); extern void test_wallet_utxo_idx_not_reused(); @@ -272,6 +273,7 @@ int main() #ifdef WITH_WALLET u_run_test(test_wallet_basics); u_run_test(test_wallet); + u_run_test(test_wallet_file_is_private); u_run_test(test_wallet_malformed_reclen); u_run_test(test_wallet_reorg_utxo_update); u_run_test(test_wallet_utxo_idx_not_reused); diff --git a/test/utils_tests.c b/test/utils_tests.c index dd382bac3..b7daa012e 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -5,6 +5,8 @@ * Distributed under the MIT software license, see the accompanying * * file COPYING or http://www.opensource.org/licenses/mit-license.php.* **********************************************************************/ +#include +#include #include #include @@ -181,4 +183,28 @@ void test_utils_fopen_private() u_assert_true(dogecoin_fopen_private(NULL, "wb") == NULL); u_assert_true(dogecoin_fopen_private(path, NULL) == NULL); #endif + + /* "r" must not create. O_RDWR|O_CREAT used to be unconditional, so asking + to open an existing file created an empty one instead of failing -- on + the wallet path that turns "open my wallet" into "start a new one". */ + remove(path); + fp = dogecoin_fopen_private(path, "rb"); + u_assert_true(fp == NULL); + u_assert_true(stat(path, &st) != 0); + + /* "wx": exclusive create, which is what replaced the access()-then-open + race in seal.c. Refuses an existing file and will not follow a symlink + planted between the two calls the old pattern needed. */ + fp = dogecoin_fopen_private(path, "wbx"); + u_assert_true(fp != NULL); + fclose(fp); + u_assert_int_eq(stat(path, &st), 0); + u_assert_int_eq((int)(st.st_mode & 07777), 0600); + + fp = dogecoin_fopen_private(path, "wbx"); + u_assert_true(fp == NULL); + u_assert_int_eq(errno, EEXIST); + + remove(path); } + diff --git a/test/wallet_tests.c b/test/wallet_tests.c index 89a812525..c7a392288 100644 --- a/test/wallet_tests.c +++ b/test/wallet_tests.c @@ -19,6 +19,7 @@ static const char *wallettmpfile = "/tmp/dummy"; #endif #endif +#include #include #include @@ -160,6 +161,39 @@ static const char * wallet_txns[] = { "020000000849afa76e800d86894dc7e6e8adf8986dc4bde2fc3c465dd549977fff5b71664a010000006b483045022100b06ebe40a3a4dde1b83a27a339c573107e2f5797ff461cc99e8e077951895d1702205da2c0ae2c05c2edb0801227fffaea9578eeda0fb831c59410eecbb5b62f8ee90121030c1a563c15d058adec64136f57454b10afb910782c2fbf783c10c9402afef068feffffffece35a2f637923635afbbcc3829704332b73a07fc941e34542c202a9a6975fd51e0000006b4830450221009254f188e3f89664cc7b3c8c2fefa8ec3195542781b029efdf9104e0cf1bc5080220150e9b558fd495b358dbe7462c94a15ead15bfcfff653114effd5021360fce29012103dee8b7607da89842428755f792b5cf683dfa239b20c3940cea85d2b7bdf2a7f2feffffffdf185aae1b0663fc0a06941fd49ed691e194fbf35d301cc750aa00b7c8daf42e000000006b483045022100cd72dac5a73afabe1d776214bc863d0643a518d1ba8062023398adc7175b25da02204c09d4200827c1696f9da13ba7bd0624cbff73b4d1f86affe70bc60e6335efb8012103cc0b3842090d0be5282b03e679839a2b6ae33f8166a8c32c91e548d673cccd81feffffff0901f6deae27dd720d456522f774aa4f98484ba02ab27d7e2f05e0d9b288d39f000000006b483045022100f26c99b2d9472fbddf7bdf305abfc2c9737eb3202a1cb8f5229fe13cde60d89d02207bfcf757bcd2850cf63544df58b521f8d4788294b214db6b9d39d940d8517871012103d5c532eeb7d17931a9d5ffa3a02ab18aa9d63a0e8b00c689b5fdb546dfb08f22feffffffdde5461b1f14d447d3a90f27395d4ebe6821e0e06e560c69b74b2593a27a516f010000006a473044022037ed0f2b2d63054d55b37688dadeebc777c6508d75957dd2e657773c7efa3e6e02204661d024d5dc27deee69dc244f662348720a088f014be311cf04dabf4a6e15ee01210367728538c117ccb63a475f6e896f1f17d87e09259fc02a8b9c47fb6c6343f2c5feffffff76e101242ef08a30654f8195771abfa2b8815cc483414d12ae5b60ecf36d4bc3010000006a4730440220664af6cb65b2613152327347f43b7dfc9015553f215d8d753d2c5d67414d94bf02200e2a36bb7dc825222fd58cc6ad99c17f76f308a690b9b1cd2790c7305dea4bd50121023c224d1dfa52f62b4bb3353f6adf7c1eea27850965ac8fb40e0a34e4701a749bfeffffff676190fcd2677ec1551ac1becad8f616d3ec7aa8d27e7e3828e40059566e4971000000006a473044022047d5afc8a060ec3e056e479287a5066a805a131dbd7ece93c9c92ddcbd130cca022047a41908c6a15603c6b23235e82fda39d27e3c17339db483dfb9cbbbdd200584012103f7402cdfa05ba65f76027f76f468d5ae302995ebc5c1d46b5aab06d6c38a8d4dfeffffffe3a6e193f057e9fb39a25504e17dd1c81b11c7c7619131f590ddbd1bdebbe784010000006a4730440220120ac2306b0aac057903da20f2a4a08a4b407cac416bd3deed015a0c998a935a0220407222f861b3972c3b57981cfedb2280428bfdffe7fb72d69c47c81a3fc978f60121038a607d3f35e020ca3d5ffe86b5eb1ed2dee0ff8918467c580d489a2bc6a36e7bfeffffff02c0bdd204000000001976a9145e6f9105b1a9100686139d0f0898b03517ad7c0588acb6f40c00000000001976a91402fbbaa336f0e82c985a58329059d2d4484f50ec88ac68860700", "010000000187e274649a4316f8ac6c9455d4423e1d66f2d26f0e9750d0989fff31b756bf9400000000da00483045022100a5fb5dbaa5de30b786e919126e6afa0724880a2d7da4e70a8110027ad7d10e9302204a60860c03c16414440fb66856ccbaac6e049838c8f886968f6ccffba7d4435b0147304402202e7bbaf5a81611c456128e573b3de0264aa8cad806ec647452d46f16c33ad00d02207625319a38665e54daad5ad1c73279636fd8869f7d833dc25717135b2b67524101475221032896838cbccf49a5269f551f0a5bb942fa854f183f23995bfb7f563add01a1942103304189fcd189d3245da6f96f4578e49f4a42eaf98dbd78d7376f627824e60fc952ae0000000003edfbd100000000001976a914e195b669de8e49f955749033fa2d79390732c43588ac21258c00000000001976a914ccac0dbbb5e80607e9167fcc1f1d07dcfcc4418b88ac3a37a3000000000017a9142e0065cd27ed91ef25c4d7c74f21d2516598b5f08700000000" }; + +/* The wallet database holds keys. It was created at the process umask, which + on a permissive one (0022) leaves it world-readable -- and the private open + added for it sat in dogecoin_wallet_create(), which never runs on the + creation path because dogecoin_wallet_load() has already set wallet->dbfile + by the time it is called. */ +void test_wallet_file_is_private() +{ +#ifndef _WIN32 + char path[] = "/tmp/dogecoin_wallet_perm_XXXXXX"; + int fd = mkstemp(path); + u_assert_true(fd >= 0); + close(fd); + unlink(path); /* we want the library to create it, not mkstemp */ + + mode_t old = umask(0022); /* the permissive case that exposed this */ + + dogecoin_wallet *wallet = dogecoin_wallet_new(&dogecoin_chainparams_main); + int error = 0; + dogecoin_bool created = false; + u_assert_int_eq(dogecoin_wallet_load(wallet, path, &error, &created, false), true); + u_assert_true(created); + + struct stat st; + u_assert_int_eq(stat(path, &st), 0); + u_assert_int_eq((int)(st.st_mode & 0777), 0600); + + dogecoin_wallet_free(wallet); + umask(old); + unlink(path); +#endif +} + void test_wallet() { // test balance of random choosen mainnet address 1MZnPNbhtmRjzAHqEikQYB7ENaRd5ky4aT From ed124270b6900d3a54cbe81f4742d0d45075a9e6 Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 17:02:18 -0700 Subject: [PATCH 3/7] test: give the file-mode tests a private directory instead of a known path CodeQL flagged four time-of-check/time-of-use races on this PR, all four in the tests added by the previous commit, none in src/. It was right about the pattern. test_utils_fopen_private() used a fixed relative name, so two runs in the same tree shared it and every stat()-then-open in the test was a genuine check-then-use against a path anyone could pre-create. The wallet test did mkstemp and then unlinked, which is worse: it publishes the name and then asks the library to create it, with a window in between. Both now allocate a 0700 directory with mkdtemp and work inside it, which removes the window rather than suppressing the alert. Assertions are unchanged -- a wallet created under umask 0022 must still land 0600, "r" must still refuse to create, and "wx" must still refuse an existing file. 83/83, clean under ASan and UBSan. --- test/utils_tests.c | 13 ++++++++++--- test/wallet_tests.c | 14 +++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/test/utils_tests.c b/test/utils_tests.c index b7daa012e..6e3a1b5b0 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -154,12 +154,18 @@ void test_dit() void test_utils_fopen_private() { #ifndef _WIN32 - const char* path = "dogecoin_fopen_private_test.tmp"; + /* A private directory of our own, rather than a fixed name in the working + directory: two test runs in the same tree would otherwise share the file, + and every stat()-then-open below would be a real check-then-use against a + path anyone can pre-create. mkdtemp gives us 0700 and a name nobody can + predict. */ + char dir[] = "/tmp/dogecoin_fopen_priv_XXXXXX"; + u_assert_true(mkdtemp(dir) != NULL); + char path[128]; + snprintf(path, sizeof(path), "%s/f.tmp", dir); struct stat st; FILE* fp; - remove(path); - fp = dogecoin_fopen_private(path, "wb"); u_assert_true(fp != NULL); fputc('x', fp); @@ -206,5 +212,6 @@ void test_utils_fopen_private() u_assert_int_eq(errno, EEXIST); remove(path); + rmdir(dir); } diff --git a/test/wallet_tests.c b/test/wallet_tests.c index c7a392288..be7778c47 100644 --- a/test/wallet_tests.c +++ b/test/wallet_tests.c @@ -170,11 +170,14 @@ static const char * wallet_txns[] = { void test_wallet_file_is_private() { #ifndef _WIN32 - char path[] = "/tmp/dogecoin_wallet_perm_XXXXXX"; - int fd = mkstemp(path); - u_assert_true(fd >= 0); - close(fd); - unlink(path); /* we want the library to create it, not mkstemp */ + /* mkdtemp, not mkstemp-then-unlink: unlinking and then asking the library + to create the same path is itself a check-then-use, and the name is + already known by then. A 0700 directory nobody else can enter makes the + creation below unambiguous. */ + char dir[] = "/tmp/dogecoin_wallet_perm_XXXXXX"; + u_assert_true(mkdtemp(dir) != NULL); + char path[128]; + snprintf(path, sizeof(path), "%s/w.db", dir); mode_t old = umask(0022); /* the permissive case that exposed this */ @@ -191,6 +194,7 @@ void test_wallet_file_is_private() dogecoin_wallet_free(wallet); umask(old); unlink(path); + rmdir(dir); #endif } From d55cf9120c3d71f212ac90aebb303575d4c72dfe Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 17:30:36 -0700 Subject: [PATCH 4/7] test: assert file modes with fstat on the open handle, not stat on the path The mkdtemp change in the previous commit did not clear the CodeQL time-of-check/time-of-use alerts, and it should not have: a private directory removes who can win the race, not the race. The tests still opened a file, closed it, and then re-resolved the same name with stat() to check its mode. Re-resolving the path asks about whatever is at that name now, which is exactly the check-then-use pattern dogecoin_fopen_private() exists to avoid -- so the tests for it were demonstrating the wrong habit. They now fstat the descriptor they already hold, which asserts the mode of the file that was actually opened. The "r must not create" case dropped its stat() as well: a second open of the same mode failing proves nothing was created without naming the path again. Assertions are unchanged in substance. A wallet created under umask 0022 must land 0600, "r" must refuse to create, "wx" must refuse an existing file, and reopening must not widen an existing mode. 83/83, clean under ASan and UBSan. --- test/utils_tests.c | 22 ++++++++++++++-------- test/wallet_tests.c | 5 ++++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/test/utils_tests.c b/test/utils_tests.c index 6e3a1b5b0..e501ddc81 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -169,19 +169,21 @@ void test_utils_fopen_private() fp = dogecoin_fopen_private(path, "wb"); u_assert_true(fp != NULL); fputc('x', fp); - fclose(fp); - - u_assert_int_eq(stat(path, &st), 0); + /* fstat the descriptor we hold, not the path. Re-resolving the name would + ask about whatever is there now rather than the file that was opened -- + the same check-then-use the function under test exists to avoid. */ + u_assert_int_eq(fstat(fileno(fp), &st), 0); /* Owner read/write only: no group or other bits at all. */ u_assert_int_eq((int)(st.st_mode & 07777), 0600); u_assert_int_eq((int)(st.st_mode & (S_IRWXG | S_IRWXO)), 0); + fclose(fp); /* Reopening must not widen the mode of a file that already exists. */ fp = dogecoin_fopen_private(path, "a+b"); u_assert_true(fp != NULL); - fclose(fp); - u_assert_int_eq(stat(path, &st), 0); + u_assert_int_eq(fstat(fileno(fp), &st), 0); u_assert_int_eq((int)(st.st_mode & 07777), 0600); + fclose(fp); remove(path); @@ -196,16 +198,20 @@ void test_utils_fopen_private() remove(path); fp = dogecoin_fopen_private(path, "rb"); u_assert_true(fp == NULL); - u_assert_true(stat(path, &st) != 0); + /* Nothing was created: a second open of the same mode still fails. Asking + stat() instead would re-resolve the path, which is the pattern being + tested against. */ + fp = dogecoin_fopen_private(path, "rb"); + u_assert_true(fp == NULL); /* "wx": exclusive create, which is what replaced the access()-then-open race in seal.c. Refuses an existing file and will not follow a symlink planted between the two calls the old pattern needed. */ fp = dogecoin_fopen_private(path, "wbx"); u_assert_true(fp != NULL); - fclose(fp); - u_assert_int_eq(stat(path, &st), 0); + u_assert_int_eq(fstat(fileno(fp), &st), 0); u_assert_int_eq((int)(st.st_mode & 07777), 0600); + fclose(fp); fp = dogecoin_fopen_private(path, "wbx"); u_assert_true(fp == NULL); diff --git a/test/wallet_tests.c b/test/wallet_tests.c index be7778c47..6e0eb38b0 100644 --- a/test/wallet_tests.c +++ b/test/wallet_tests.c @@ -187,8 +187,11 @@ void test_wallet_file_is_private() u_assert_int_eq(dogecoin_wallet_load(wallet, path, &error, &created, false), true); u_assert_true(created); + /* fstat the handle the wallet is holding rather than re-resolving the + path: this asserts the mode of the file that was actually created. */ struct stat st; - u_assert_int_eq(stat(path, &st), 0); + u_assert_true(wallet->dbfile != NULL); + u_assert_int_eq(fstat(fileno(wallet->dbfile), &st), 0); u_assert_int_eq((int)(st.st_mode & 0777), 0600); dogecoin_wallet_free(wallet); From 2f8ffc89efdcd383e2dc4b495c40e61cd8081736 Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 18:02:40 -0700 Subject: [PATCH 5/7] test: keep the file-mode tests off MSVC and off Android's missing /tmp Three portability faults in the tests added by the previous commits, all mine, all found by CI rather than locally. utils_tests.c included unconditionally, which MSVC does not have: error C1083 on every Windows job. The file already guards that way; this one was added without looking. The new assertions in test_utils_fopen_private() were appended after the closing #endif of the function's #ifndef _WIN32 block, so on Windows they referenced path, fp, st and dir -- all declared inside it. The guard now closes at the end of the function, which is where it belonged. Both tests allocated their scratch directory under /tmp. Android has no writable /tmp, which is why wallet_tests.c already picks /data/local/tmp for wallettmpfile; the new code ignored that and the aarch64-android job exited 2 after the last passing test. Both now derive the base the same way. No assertion changed. 83/83 locally. --- test/utils_tests.c | 18 ++++++++++++++++-- test/wallet_tests.c | 12 +++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/test/utils_tests.c b/test/utils_tests.c index e501ddc81..eddfe3e47 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -6,9 +6,23 @@ * file COPYING or http://www.opensource.org/licenses/mit-license.php.* **********************************************************************/ #include +#ifndef _WIN32 #include +#endif #include +#ifndef _WIN32 +/* Android has no writable /tmp; the rest of this suite already special-cases + it for wallettmpfile. */ +#ifdef __ANDROID__ +#define DOGECOIN_TEST_TMPDIR "/data/local/tmp" +#else +#define DOGECOIN_TEST_TMPDIR "/tmp" +#endif +#endif + + + #include #ifndef _WIN32 #include @@ -159,7 +173,7 @@ void test_utils_fopen_private() and every stat()-then-open below would be a real check-then-use against a path anyone can pre-create. mkdtemp gives us 0700 and a name nobody can predict. */ - char dir[] = "/tmp/dogecoin_fopen_priv_XXXXXX"; + char dir[] = DOGECOIN_TEST_TMPDIR "/dogecoin_fopen_priv_XXXXXX"; u_assert_true(mkdtemp(dir) != NULL); char path[128]; snprintf(path, sizeof(path), "%s/f.tmp", dir); @@ -190,7 +204,6 @@ void test_utils_fopen_private() /* NULL arguments are refused rather than passed through. */ u_assert_true(dogecoin_fopen_private(NULL, "wb") == NULL); u_assert_true(dogecoin_fopen_private(path, NULL) == NULL); -#endif /* "r" must not create. O_RDWR|O_CREAT used to be unconditional, so asking to open an existing file created an empty one instead of failing -- on @@ -219,5 +232,6 @@ void test_utils_fopen_private() remove(path); rmdir(dir); +#endif /* _WIN32 */ } diff --git a/test/wallet_tests.c b/test/wallet_tests.c index 6e0eb38b0..5f8b3b5e2 100644 --- a/test/wallet_tests.c +++ b/test/wallet_tests.c @@ -19,6 +19,16 @@ static const char *wallettmpfile = "/tmp/dummy"; #endif #endif +#ifndef _WIN32 +/* Android has no writable /tmp; the rest of this suite already special-cases + it for wallettmpfile. */ +#ifdef __ANDROID__ +#define DOGECOIN_TEST_TMPDIR "/data/local/tmp" +#else +#define DOGECOIN_TEST_TMPDIR "/tmp" +#endif +#endif + #include #include @@ -174,7 +184,7 @@ void test_wallet_file_is_private() to create the same path is itself a check-then-use, and the name is already known by then. A 0700 directory nobody else can enter makes the creation below unambiguous. */ - char dir[] = "/tmp/dogecoin_wallet_perm_XXXXXX"; + char dir[] = DOGECOIN_TEST_TMPDIR "/dogecoin_wallet_perm_XXXXXX"; u_assert_true(mkdtemp(dir) != NULL); char path[128]; snprintf(path, sizeof(path), "%s/w.db", dir); From d4c801f8fd12ad75d71981e30a5aede796a0a334 Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 19:40:16 -0700 Subject: [PATCH 6/7] utils: dogecoin_fopen_private has no filesystem to open under OP-TEE The aarch64-linux-optee job failed at link: libdogecoin.a(libdogecoin_la-utils.o): in function `dogecoin_fopen_private': utils.c:(.text+0x20): undefined reference to `open' utils.c:(.text+0x30): undefined reference to `fdopen' utils.c:(.text+0x40): undefined reference to `close' A trusted application has no filesystem, so those are not provided. Nothing in the TA calls this function -- referencing the symbols is enough to fail the link. Guarded the same way the rest of utils.c guards its filesystem code, returning NULL. Both callers already handle that: dogecoin_wallet_create() sets *error and returns false, and the seal.c sites test the pointer before use. Verified by compiling utils.c with -DUSE_OPTEE and checking the object: no undefined reference to open, fdopen or close remains. 83/83 on the normal build. --- src/utils.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/utils.c b/src/utils.c index 74f4f2413..9ff4c84f6 100644 --- a/src/utils.c +++ b/src/utils.c @@ -686,7 +686,15 @@ char* concat(char* prefix, char* suffix) { FILE* dogecoin_fopen_private(const char* path, const char* mode) { -#ifdef _WIN32 +#ifdef USE_OPTEE + /* A trusted application has no filesystem: open(), fdopen() and close() + are not provided, and referencing them fails the TA link even though + nothing in the TA calls this. The rest of utils.c guards its filesystem + code the same way. */ + (void)path; + (void)mode; + return NULL; +#elif defined(_WIN32) /* No umask on Windows; a new file inherits the directory's ACL, so plain fopen already gets whatever the parent grants. */ return fopen(path, mode); From cd188a8a9f77023d766270ebe27c52af2d9dd648 Mon Sep 17 00:00:00 2001 From: bluezr Date: Sat, 8 Aug 2026 20:31:22 -0700 Subject: [PATCH 7/7] utils: fall back to fopen under OP-TEE rather than returning NULL The previous commit stubbed dogecoin_fopen_private to NULL under USE_OPTEE to fix the TA link, which fixed the link and broke the tests: the same library is linked for the host side of an OP-TEE build, where wallet.c really does open files, so every call started failing. wallet.c:1155 has always called plain fopen and the TA has always linked, which says what the TA actually provides: fopen, but not the POSIX open/fdopen/close this function is built from. So fall back to fopen rather than refuse. Windows takes the same path for its own reason -- no umask there, a new file inherits the directory ACL, and plain fopen already gets whatever the parent grants. Neither fallback delivers the 0600-on-create guarantee, so the tests that assert it are compiled out on both. On OP-TEE that costs nothing: the TA has no filesystem to guarantee permissions for. Verified by compiling utils.c with -DUSE_OPTEE: no undefined reference to open, fdopen or close remains, which was the original link failure. 83/83. --- src/utils.c | 22 +++++++++++----------- test/utils_tests.c | 5 ++++- test/wallet_tests.c | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/utils.c b/src/utils.c index 9ff4c84f6..7b8f46673 100644 --- a/src/utils.c +++ b/src/utils.c @@ -686,17 +686,17 @@ char* concat(char* prefix, char* suffix) { FILE* dogecoin_fopen_private(const char* path, const char* mode) { -#ifdef USE_OPTEE - /* A trusted application has no filesystem: open(), fdopen() and close() - are not provided, and referencing them fails the TA link even though - nothing in the TA calls this. The rest of utils.c guards its filesystem - code the same way. */ - (void)path; - (void)mode; - return NULL; -#elif defined(_WIN32) - /* No umask on Windows; a new file inherits the directory's ACL, so plain - fopen already gets whatever the parent grants. */ +#if defined(USE_OPTEE) || defined(_WIN32) + /* OP-TEE provides fopen but not the POSIX open/fdopen/close this uses -- + referencing them fails the TA link even though nothing in the TA calls + this function. Fall back rather than return NULL: the same library is + linked for the host side of an OP-TEE build, where wallet.c does open + files, and stubbing this out made those calls fail and took the mode + assertions with them. + Windows has no umask, so a new file inherits the directory ACL and plain + fopen already gets whatever the parent grants. + Neither platform gets the 0600-on-create guarantee; on OP-TEE the TA has + no filesystem to guarantee it for. */ return fopen(path, mode); #else int flags; diff --git a/test/utils_tests.c b/test/utils_tests.c index eddfe3e47..61c98fccd 100644 --- a/test/utils_tests.c +++ b/test/utils_tests.c @@ -167,7 +167,10 @@ void test_dit() */ void test_utils_fopen_private() { -#ifndef _WIN32 +/* The 0600-on-create guarantee only exists on the POSIX path. Windows has no + umask, and OP-TEE has no filesystem in the TA and no open/fdopen/close to + build it from, so both fall back to plain fopen. */ +#if !defined(_WIN32) && !defined(USE_OPTEE) /* A private directory of our own, rather than a fixed name in the working directory: two test runs in the same tree would otherwise share the file, and every stat()-then-open below would be a real check-then-use against a diff --git a/test/wallet_tests.c b/test/wallet_tests.c index 5f8b3b5e2..e0f823737 100644 --- a/test/wallet_tests.c +++ b/test/wallet_tests.c @@ -179,7 +179,7 @@ static const char * wallet_txns[] = { by the time it is called. */ void test_wallet_file_is_private() { -#ifndef _WIN32 +#if !defined(_WIN32) && !defined(USE_OPTEE) /* mkdtemp, not mkstemp-then-unlink: unlinking and then asking the library to create the same path is itself a check-then-use, and the name is already known by then. A 0700 directory nobody else can enter makes the