Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/headersdb_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,10 @@ dogecoin_blockindex * dogecoin_headers_db_connect_hdr(dogecoin_headers_db* db, s
// keep them only on-disk
dogecoin_blockindex *scan_tip = db->chaintip;
unsigned int i;
for (i = 0; i < db->max_hdr_in_mem + 1; i++)
/* scan_tip was dereferenced below before the `scan_tip &&` check
further down had a chance to run. chaintip is NULL until the
first header is connected. */
for (i = 0; scan_tip && i < db->max_hdr_in_mem + 1; i++)
{
if (scan_tip->prev) {
scan_tip = scan_tip->prev;
Expand Down
9 changes: 9 additions & 0 deletions src/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,11 @@ void print_header(char* filepath) {

if ((fptr = fopen(filename, "r")) == NULL)
{
/* The error branch used to fall through: print_image() then ran fgets()
on a NULL FILE*, and fclose(NULL) followed it. Opening a file that is
not there is ordinary, so this crashed on ordinary input. */
fprintf(stderr, "error opening %s\n", filename);
return;
}

print_image(fptr);
Expand All @@ -610,6 +614,11 @@ void print_image(FILE* fptr)
#ifndef USE_OPTEE // OPTEE has no filesystem or console
char read_string[MAX_LEN];

/* Exported, so a caller outside this file can reach it with NULL too. */
if (!fptr) {
return;
}

while (fgets(read_string, sizeof(read_string), fptr) != NULL)
printf("%s", read_string);
#else
Expand Down
14 changes: 10 additions & 4 deletions src/wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -1722,6 +1722,9 @@ int64_t dogecoin_wallet_get_debit_txi(dogecoin_wallet *wallet, const dogecoin_tx
int64_t dogecoin_wallet_get_debit_tx(dogecoin_wallet *wallet, const dogecoin_tx *tx) {
unsigned int i;
int64_t debit = 0;
if (!wallet || !tx) {
return 0;
}
if (tx->vin) {
for (i = 0; i < tx->vin->len; i++) {
dogecoin_tx_in* tx_in= vector_idx(tx->vin, i);
Expand All @@ -1735,14 +1738,17 @@ int64_t dogecoin_wallet_get_debit_tx(dogecoin_wallet *wallet, const dogecoin_tx

dogecoin_bool dogecoin_wallet_is_from_me(dogecoin_wallet *wallet, const dogecoin_tx *tx)
{
if (dogecoin_wallet_get_debit_tx(wallet, tx) > 0) {
return true;
}

/* This guard used to sit below the get_debit_tx() call, which dereferences
tx immediately, so a NULL tx crashed four lines before the check that
exists to catch it. */
if (!wallet || !tx || !tx->vin) {
return false;
}

if (dogecoin_wallet_get_debit_tx(wallet, tx) > 0) {
return true;
}

for (unsigned int i = 0; i < tx->vin->len; i++) {
dogecoin_tx_in* tx_in = vector_idx(tx->vin, i);
if (!tx_in) {
Expand Down
4 changes: 4 additions & 0 deletions test/unittester.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ extern void test_invalid_tx_deser();
extern void test_tx_sign();
extern void test_scripts();
extern void test_utils();
extern void test_utils_null_file_guards();
extern void test_vector();
extern void test_qr();

Expand Down Expand Up @@ -131,6 +132,7 @@ extern void test_examples();
#ifdef WITH_WALLET
extern void test_wallet_basics();
extern void test_wallet();
extern void test_wallet_null_tx_guards();
extern void test_wallet_malformed_reclen();
extern void test_wallet_reorg_utxo_update();
extern void test_wallet_utxo_idx_not_reused();
Expand Down Expand Up @@ -232,6 +234,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_null_file_guards);
u_run_test(test_vector);
u_run_test(test_qr);

Expand Down Expand Up @@ -266,6 +269,7 @@ int main()
#ifdef WITH_WALLET
u_run_test(test_wallet_basics);
u_run_test(test_wallet);
u_run_test(test_wallet_null_tx_guards);
u_run_test(test_wallet_malformed_reclen);
u_run_test(test_wallet_reorg_utxo_update);
u_run_test(test_wallet_utxo_idx_not_reused);
Expand Down
20 changes: 20 additions & 0 deletions test/utils_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,23 @@ void test_dit()
debug_print("%s", "DIT test: disabled (DIT not supported)\n");
}
}


/*
* print_header() ignored a failed fopen: the error branch printed a message and
* fell through to print_image(), which ran fgets() on a NULL FILE*, and then to
* fclose(NULL). Opening a file that is not there is ordinary input, so this
* crashed on ordinary input. print_image() is LIBDOGECOIN_API, so it is
* reachable with NULL from outside this file as well.
*/
void test_utils_null_file_guards()
{
/* Must return quietly rather than dereferencing the NULL FILE*. */
print_image(NULL);

/* A path that cannot be opened must not crash. */
print_header("this-path-does-not-exist-libdogecoin-test");

/* NULL path was already guarded; assert it stays that way. */
print_header(NULL);
}
37 changes: 37 additions & 0 deletions test/wallet_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ static const char *wallettmpfile = "/tmp/dummy";
#include <dogecoin/base58.h>
#include <dogecoin/ecc.h>
#include <dogecoin/utils.h>
#include <dogecoin/tx.h>
#include <dogecoin/wallet.h>
#include <dogecoin/script.h>
#if !defined(_WIN32)
Expand Down Expand Up @@ -559,3 +560,39 @@ void test_wallet_balance_accounts_for_spends() {
dogecoin_wallet_free(wallet);
remove_all_utxos();
}


/*
* Null guards that sat below the dereference they were meant to guard.
*
* dogecoin_wallet_is_from_me() called dogecoin_wallet_get_debit_tx() first,
* and that function opens with `if (tx->vin)`. The `!wallet || !tx` check came
* four lines later, so a NULL tx crashed before reaching it. Both functions are
* LIBDOGECOIN_API, and rest.c passes wtx->tx to is_from_me without checking it
* (wallet.c:954 does check, which is why this survived).
*
* Without the fix these dereference NULL rather than failing an assertion.
*/
void test_wallet_null_tx_guards()
{
dogecoin_wallet* wallet = dogecoin_wallet_new(&dogecoin_chainparams_main);
u_assert_true(wallet != NULL);

/* The call that used to crash. */
u_assert_int_eq((int)dogecoin_wallet_is_from_me(wallet, NULL), 0);

/* The function underneath it, reachable directly -- rest.c:184 calls it. */
u_assert_true(dogecoin_wallet_get_debit_tx(wallet, NULL) == 0);

/* A NULL wallet must be refused too, not just a NULL tx. */
u_assert_int_eq((int)dogecoin_wallet_is_from_me(NULL, NULL), 0);
u_assert_true(dogecoin_wallet_get_debit_tx(NULL, NULL) == 0);

/* A well-formed but empty tx still answers false rather than crashing. */
dogecoin_tx* tx = dogecoin_tx_new();
u_assert_true(tx != NULL);
u_assert_int_eq((int)dogecoin_wallet_is_from_me(wallet, tx), 0);
dogecoin_tx_free(tx);

dogecoin_wallet_free(wallet);
}
Loading