From 7405987ebb23dc642fd1499979690c70097f1630 Mon Sep 17 00:00:00 2001 From: zhenglong li Date: Tue, 18 Aug 2026 18:16:39 +0800 Subject: [PATCH] pgcrypto: Ensure debug handler is reset on error in PGP functions The PGP encryption and decryption functions install a global debug handler when the "debug=1" option is given, and relied on every error path explicitly resetting it before throwing. Commit d0ecee6de9a added an ereport() call in cfb_process() which can fire while the handler is installed, for example when a cipher fails its deferred initialization under OpenSSL running in FIPS mode or without the legacy provider loaded. The error would leave the handler installed for the remainder of the backend's lifetime, causing subsequent PGP calls in the same backend to emit "dbg:" NOTICE messages even without the debug option, until some PGP call happened to complete normally. Rather than adding yet another explicit reset at the new error site, wrap the bodies of encrypt_internal() and decrypt_internal() in PG_TRY/PG_FINALLY so that the handler is reset no matter how we exit. This also closes preexisting windows of the same kind, such as an encoding conversion error or out-of-memory failure occurring while the handler is installed, and lets us remove the reset calls that were previously scattered across the success and error paths. No memory or resource cleanup needs to be moved into the PG_FINALLY block: all allocations are palloc-based and OpenSSL handles are tracked by ResourceOwner, so error recovery already takes care of those. --- contrib/pgcrypto/pgp-pgsql.c | 240 +++++++++++++++++++---------------- 1 file changed, 131 insertions(+), 109 deletions(-) diff --git a/contrib/pgcrypto/pgp-pgsql.c b/contrib/pgcrypto/pgp-pgsql.c index a4b1eac71bf..f174c5d8261 100644 --- a/contrib/pgcrypto/pgp-pgsql.c +++ b/contrib/pgcrypto/pgp-pgsql.c @@ -378,7 +378,7 @@ encrypt_internal(int is_pubenc, int is_text, *dst; uint8 tmp[VARHDRSZ]; uint8 *restmp; - bytea *res; + bytea *volatile res = NULL; int res_len; PGP_Context *ctx; int err; @@ -387,71 +387,81 @@ encrypt_internal(int is_pubenc, int is_text, init_work(&ctx, is_text, args, &ex); - if (is_text && pgp_get_unicode_mode(ctx)) + /* + * init_work() may have installed the global debug handler. Make sure it + * gets reset however we exit, lest an error thrown below (for example a + * cipher failure reported by cfb_process(), or an encoding conversion + * error) leave it installed for the remainder of the backend's lifetime. + */ + PG_TRY(); { - tmp_data = convert_to_utf8(data); - if (tmp_data == data) - tmp_data = NULL; - else - data = tmp_data; - } + if (is_text && pgp_get_unicode_mode(ctx)) + { + tmp_data = convert_to_utf8(data); + if (tmp_data == data) + tmp_data = NULL; + else + data = tmp_data; + } - src = create_mbuf_from_vardata(data); - dst = mbuf_create(VARSIZE_ANY(data) + 128); + src = create_mbuf_from_vardata(data); + dst = mbuf_create(VARSIZE_ANY(data) + 128); - /* - * reserve room for header - */ - mbuf_append(dst, tmp, VARHDRSZ); + /* + * reserve room for header + */ + mbuf_append(dst, tmp, VARHDRSZ); - /* - * set key - */ - if (is_pubenc) - { - MBuf *kbuf = create_mbuf_from_vardata(key); + /* + * set key + */ + if (is_pubenc) + { + MBuf *kbuf = create_mbuf_from_vardata(key); - err = pgp_set_pubkey(ctx, kbuf, - NULL, 0, 0); - mbuf_free(kbuf); - } - else - err = pgp_set_symkey(ctx, (uint8 *) VARDATA_ANY(key), - VARSIZE_ANY_EXHDR(key)); + err = pgp_set_pubkey(ctx, kbuf, + NULL, 0, 0); + mbuf_free(kbuf); + } + else + err = pgp_set_symkey(ctx, (uint8 *) VARDATA_ANY(key), + VARSIZE_ANY_EXHDR(key)); + + /* + * encrypt + */ + if (err >= 0) + err = pgp_encrypt(ctx, src, dst); + + /* + * check for error + */ + if (err) + { + if (tmp_data) + clear_and_pfree(tmp_data); + pgp_free(ctx); + mbuf_free(src); + mbuf_free(dst); + px_THROW_ERROR(err); + } - /* - * encrypt - */ - if (err >= 0) - err = pgp_encrypt(ctx, src, dst); + /* res_len includes VARHDRSZ */ + res_len = mbuf_steal_data(dst, &restmp); + res = (bytea *) restmp; + SET_VARSIZE(res, res_len); - /* - * check for error - */ - if (err) - { - if (ex.debug) - px_set_debug_handler(NULL); if (tmp_data) clear_and_pfree(tmp_data); pgp_free(ctx); mbuf_free(src); mbuf_free(dst); - px_THROW_ERROR(err); } - - /* res_len includes VARHDRSZ */ - res_len = mbuf_steal_data(dst, &restmp); - res = (bytea *) restmp; - SET_VARSIZE(res, res_len); - - if (tmp_data) - clear_and_pfree(tmp_data); - pgp_free(ctx); - mbuf_free(src); - mbuf_free(dst); - - px_set_debug_handler(NULL); + PG_FINALLY(); + { + px_set_debug_handler(NULL); + } + PG_END_TRY(); return res; } @@ -465,7 +475,7 @@ decrypt_internal(int is_pubenc, int need_text, text *data, *dst = NULL; uint8 tmp[VARHDRSZ]; uint8 *restmp; - bytea *res; + bytea *volatile res = NULL; int res_len; PGP_Context *ctx = NULL; struct debug_expect ex; @@ -474,77 +484,89 @@ decrypt_internal(int is_pubenc, int need_text, text *data, init_work(&ctx, need_text, args, &ex); - src = mbuf_create_from_data((uint8 *) VARDATA_ANY(data), - VARSIZE_ANY_EXHDR(data)); - dst = mbuf_create(VARSIZE_ANY(data) + 2048); - - /* - * reserve room for header - */ - mbuf_append(dst, tmp, VARHDRSZ); - /* - * set key + * init_work() may have installed the global debug handler. Make sure it + * gets reset however we exit, lest an error thrown below (for example a + * cipher failure reported by cfb_process(), or an encoding conversion + * error) leave it installed for the remainder of the backend's lifetime. */ - if (is_pubenc) + PG_TRY(); { - uint8 *psw = NULL; - int psw_len = 0; - MBuf *kbuf; - - if (keypsw) + src = mbuf_create_from_data((uint8 *) VARDATA_ANY(data), + VARSIZE_ANY_EXHDR(data)); + dst = mbuf_create(VARSIZE_ANY(data) + 2048); + + /* + * reserve room for header + */ + mbuf_append(dst, tmp, VARHDRSZ); + + /* + * set key + */ + if (is_pubenc) { - psw = (uint8 *) VARDATA_ANY(keypsw); - psw_len = VARSIZE_ANY_EXHDR(keypsw); + uint8 *psw = NULL; + int psw_len = 0; + MBuf *kbuf; + + if (keypsw) + { + psw = (uint8 *) VARDATA_ANY(keypsw); + psw_len = VARSIZE_ANY_EXHDR(keypsw); + } + kbuf = create_mbuf_from_vardata(key); + err = pgp_set_pubkey(ctx, kbuf, psw, psw_len, 1); + mbuf_free(kbuf); } - kbuf = create_mbuf_from_vardata(key); - err = pgp_set_pubkey(ctx, kbuf, psw, psw_len, 1); - mbuf_free(kbuf); - } - else - err = pgp_set_symkey(ctx, (uint8 *) VARDATA_ANY(key), - VARSIZE_ANY_EXHDR(key)); - - /* decrypt */ - if (err >= 0) - { - err = pgp_decrypt(ctx, src, dst); + else + err = pgp_set_symkey(ctx, (uint8 *) VARDATA_ANY(key), + VARSIZE_ANY_EXHDR(key)); - if (ex.expect) - check_expect(ctx, &ex); + /* decrypt */ + if (err >= 0) + { + err = pgp_decrypt(ctx, src, dst); - /* remember the setting */ - got_unicode = pgp_get_unicode_mode(ctx); - } + if (ex.expect) + check_expect(ctx, &ex); - mbuf_free(src); - pgp_free(ctx); + /* remember the setting */ + got_unicode = pgp_get_unicode_mode(ctx); + } - if (err) - { - px_set_debug_handler(NULL); - mbuf_free(dst); - px_THROW_ERROR(err); - } + mbuf_free(src); + pgp_free(ctx); - res_len = mbuf_steal_data(dst, &restmp); - mbuf_free(dst); + if (err) + { + mbuf_free(dst); + px_THROW_ERROR(err); + } - /* res_len includes VARHDRSZ */ - res = (bytea *) restmp; - SET_VARSIZE(res, res_len); + res_len = mbuf_steal_data(dst, &restmp); + mbuf_free(dst); - if (need_text && got_unicode) - { - text *utf = convert_from_utf8(res); + /* res_len includes VARHDRSZ */ + res = (bytea *) restmp; + SET_VARSIZE(res, res_len); - if (utf != res) + if (need_text && got_unicode) { - clear_and_pfree(res); - res = utf; + text *utf = convert_from_utf8(res); + + if (utf != res) + { + clear_and_pfree(res); + res = utf; + } } } - px_set_debug_handler(NULL); + PG_FINALLY(); + { + px_set_debug_handler(NULL); + } + PG_END_TRY(); return res; } -- 2.50.1 (Apple Git-155)