From 05c86b43b6276f75ed9ccb26f78294f66f59fef9 Mon Sep 17 00:00:00 2001 From: Yuriy Grigoryev Date: Mon, 17 Aug 2026 12:23:36 +0700 Subject: [PATCH v1] Fix use-after-free after failed pg_checksum_init. pg_checksum_init() frees the SHA context if pg_cryptohash_init() fails, but leaves a dangling pointer in the checksum context. pg_combinebackup ignores that failure in several places and later passes the pointer to pg_checksum_update() or pg_checksum_final(). NULL the pointer after freeing it, and also free the SHA context if pg_cryptohash_final() fails so that this error path does not leak. Check the return values of pg_checksum_init() and pg_checksum_final() in pg_combinebackup, matching basebackup and pg_verifybackup. Author: Yuriy Grigoryev Co-authored-by: Kanatbek Kanybekov --- src/bin/pg_combinebackup/backup_label.c | 9 ++++++-- src/bin/pg_combinebackup/pg_combinebackup.c | 11 +++++++--- src/bin/pg_combinebackup/reconstruct.c | 7 +++++- src/bin/pg_combinebackup/write_manifest.c | 7 +++++- src/common/checksum_helper.c | 24 +++++++++++++++++---- 5 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.c index e676249247d..648161edbfe 100644 --- a/src/bin/pg_combinebackup/backup_label.c +++ b/src/bin/pg_combinebackup/backup_label.c @@ -133,10 +133,12 @@ write_backup_label(char *output_directory, StringInfo buf, uint8 checksum_payload[PG_CHECKSUM_MAX_LENGTH]; int checksum_length; - pg_checksum_init(&checksum_ctx, checksum_type); - snprintf(output_filename, MAXPGPATH, "%s/backup_label", output_directory); + if (pg_checksum_init(&checksum_ctx, checksum_type) < 0) + pg_fatal("could not initialize checksum of file \"%s\"", + output_filename); + if ((output_fd = open(output_filename, O_WRONLY | O_CREAT | O_EXCL | PG_BINARY, pg_file_create_mode)) < 0) @@ -174,6 +176,9 @@ write_backup_label(char *output_directory, StringInfo buf, pg_fatal("could not close file \"%s\": %m", output_filename); checksum_length = pg_checksum_final(&checksum_ctx, checksum_payload); + if (checksum_length < 0) + pg_fatal("could not finalize checksum of file \"%s\"", + output_filename); if (mwriter != NULL) { diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 7b2d0db8768..ded36391593 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -1086,6 +1086,8 @@ process_directory_recursively(Oid tsoid, } } + snprintf(ofullpath, MAXPGPATH, "%s/%s", ofulldir, de->d_name); + /* * If we're reusing a checksum, then we don't need copy_file() to * compute one for us, but otherwise, it needs to compute whatever @@ -1093,11 +1095,11 @@ process_directory_recursively(Oid tsoid, */ if (checksum_length != 0) pg_checksum_init(&checksum_ctx, CHECKSUM_TYPE_NONE); - else - pg_checksum_init(&checksum_ctx, checksum_type); + else if (pg_checksum_init(&checksum_ctx, checksum_type) < 0) + pg_fatal("could not initialize checksum of file \"%s\"", + ofullpath); /* Actually copy the file. */ - snprintf(ofullpath, MAXPGPATH, "%s/%s", ofulldir, de->d_name); copy_file(ifullpath, ofullpath, &checksum_ctx, opt->copy_method, opt->dry_run); @@ -1111,6 +1113,9 @@ process_directory_recursively(Oid tsoid, checksum_payload = pg_malloc(PG_CHECKSUM_MAX_LENGTH); checksum_length = pg_checksum_final(&checksum_ctx, checksum_payload); + if (checksum_length < 0) + pg_fatal("could not finalize checksum of file \"%s\"", + ofullpath); } } diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c index fdb57d9d061..fd31786760d 100644 --- a/src/bin/pg_combinebackup/reconstruct.c +++ b/src/bin/pg_combinebackup/reconstruct.c @@ -318,7 +318,9 @@ reconstruct_from_incremental_file(char *input_filename, } /* Prepare for checksum calculation, if required. */ - pg_checksum_init(&checksum_ctx, checksum_type); + if (pg_checksum_init(&checksum_ctx, checksum_type) < 0) + pg_fatal("could not initialize checksum of file \"%s\"", + output_filename); /* * If the full file can be created by copying a file from an older backup @@ -354,6 +356,9 @@ reconstruct_from_incremental_file(char *input_filename, *checksum_payload = pg_malloc(PG_CHECKSUM_MAX_LENGTH); *checksum_length = pg_checksum_final(&checksum_ctx, *checksum_payload); + if (*checksum_length < 0) + pg_fatal("could not finalize checksum of file \"%s\"", + output_filename); } /* diff --git a/src/bin/pg_combinebackup/write_manifest.c b/src/bin/pg_combinebackup/write_manifest.c index 369d6d2071c..4042d9b43ee 100644 --- a/src/bin/pg_combinebackup/write_manifest.c +++ b/src/bin/pg_combinebackup/write_manifest.c @@ -54,7 +54,9 @@ create_manifest_writer(char *directory, uint64 system_identifier) initStringInfo(&mwriter->buf); mwriter->first_file = true; mwriter->still_checksumming = true; - pg_checksum_init(&mwriter->manifest_ctx, CHECKSUM_TYPE_SHA256); + if (pg_checksum_init(&mwriter->manifest_ctx, CHECKSUM_TYPE_SHA256) < 0) + pg_fatal("could not initialize checksum of file \"%s\"", + mwriter->pathname); appendStringInfo(&mwriter->buf, "{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n" @@ -174,6 +176,9 @@ finalize_manifest(manifest_writer *mwriter, appendStringInfoString(&mwriter->buf, "\"Manifest-Checksum\": \""); enlargeStringInfo(&mwriter->buf, 2 * PG_SHA256_DIGEST_STRING_LENGTH); len = pg_checksum_final(&mwriter->manifest_ctx, checksumbuf); + if (len < 0) + pg_fatal("could not finalize checksum of file \"%s\"", + mwriter->pathname); Assert(len == PG_SHA256_DIGEST_LENGTH); mwriter->buf.len += hex_encode(checksumbuf, len, &mwriter->buf.data[mwriter->buf.len]); diff --git a/src/common/checksum_helper.c b/src/common/checksum_helper.c index cc111401aeb..978ba1fe57d 100644 --- a/src/common/checksum_helper.c +++ b/src/common/checksum_helper.c @@ -99,6 +99,7 @@ pg_checksum_init(pg_checksum_context *context, pg_checksum_type type) if (pg_cryptohash_init(context->raw_context.c_sha2) < 0) { pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; return -1; } break; @@ -109,6 +110,7 @@ pg_checksum_init(pg_checksum_context *context, pg_checksum_type type) if (pg_cryptohash_init(context->raw_context.c_sha2) < 0) { pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; return -1; } break; @@ -119,6 +121,7 @@ pg_checksum_init(pg_checksum_context *context, pg_checksum_type type) if (pg_cryptohash_init(context->raw_context.c_sha2) < 0) { pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; return -1; } break; @@ -129,6 +132,7 @@ pg_checksum_init(pg_checksum_context *context, pg_checksum_type type) if (pg_cryptohash_init(context->raw_context.c_sha2) < 0) { pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; return -1; } break; @@ -201,29 +205,41 @@ pg_checksum_final(pg_checksum_context *context, uint8 *output) retval = PG_SHA224_DIGEST_LENGTH; if (pg_cryptohash_final(context->raw_context.c_sha2, output, retval) < 0) - return -1; + retval = -1; pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; + if (retval < 0) + return -1; break; case CHECKSUM_TYPE_SHA256: retval = PG_SHA256_DIGEST_LENGTH; if (pg_cryptohash_final(context->raw_context.c_sha2, output, retval) < 0) - return -1; + retval = -1; pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; + if (retval < 0) + return -1; break; case CHECKSUM_TYPE_SHA384: retval = PG_SHA384_DIGEST_LENGTH; if (pg_cryptohash_final(context->raw_context.c_sha2, output, retval) < 0) - return -1; + retval = -1; pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; + if (retval < 0) + return -1; break; case CHECKSUM_TYPE_SHA512: retval = PG_SHA512_DIGEST_LENGTH; if (pg_cryptohash_final(context->raw_context.c_sha2, output, retval) < 0) - return -1; + retval = -1; pg_cryptohash_free(context->raw_context.c_sha2); + context->raw_context.c_sha2 = NULL; + if (retval < 0) + return -1; break; } -- 2.50.1 (Apple Git-155)