From: Vsevolod Stakhov <vsevolod@rspamd.com>
Date: Sat, 4 Oct 2025 14:48:05 +0000 (+0100)
Subject: [Fix] Fix union handling in ED25519 key loading to prevent memory corruption
X-Git-Tag: 3.13.2~4^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e98518703762eabeffc0326bca02d44b15041d0b;p=thirdparty%2Frspamd.git

[Fix] Fix union handling in ED25519 key loading to prevent memory corruption

When loading ED25519 keys from PEM, the code was writing to key_eddsa in the
union and then attempting to free key_ssl pointers, which corrupted the
key_eddsa pointer and caused use-after-free/double-free during cleanup.

The fix saves the EVP_PKEY and BIO pointers to temporary variables, extracts
the raw key, frees the OpenSSL objects, and only then assigns to the union.
This prevents memory corruption and resource leaks.
---

diff --git a/src/libserver/dkim.c b/src/libserver/dkim.c
index 9587176ce2..b9492b929a 100644
--- a/src/libserver/dkim.c
+++ b/src/libserver/dkim.c
@@ -3391,21 +3391,26 @@ rspamd_dkim_sign_key_load(const char *key, size_t len,
 			nkey->keylen = EVP_PKEY_size(nkey->specific.key_ssl.key_evp);
 			break;
 #ifdef HAVE_ED25519
-		case EVP_PKEY_ED25519:
-			/* For Ed25519, extract the raw key and store it in the eddsa field */
-			nkey->type = RSPAMD_DKIM_KEY_EDDSA;
-			nkey->keylen = crypto_sign_secretkeybytes();
-			nkey->specific.key_eddsa = g_malloc(nkey->keylen);
-
-			/* Extract raw private key from EVP_PKEY */
-			size_t extracted_len = nkey->keylen;
-			if (EVP_PKEY_get_raw_private_key(nkey->specific.key_ssl.key_evp,
-											 nkey->specific.key_eddsa, &extracted_len) != 1) {
+		case EVP_PKEY_ED25519: {
+			/* For Ed25519, extract the raw key and store it in the eddsa field.
+			 * Important: specific is a union, so we must save key_ssl pointers
+			 * before overwriting them with key_eddsa. */
+			EVP_PKEY *evp_temp = nkey->specific.key_ssl.key_evp;
+			BIO *bio_temp = nkey->specific.key_ssl.key_bio;
+			unsigned char *raw_key = NULL;
+			size_t raw_key_len = crypto_sign_secretkeybytes();
+
+			/* Extract raw private key from EVP_PKEY into temporary buffer */
+			raw_key = g_malloc(raw_key_len);
+			size_t extracted_len = raw_key_len;
+			if (EVP_PKEY_get_raw_private_key(evp_temp, raw_key, &extracted_len) != 1) {
 				g_set_error(err, dkim_error_quark(), DKIM_SIGERROR_KEYFAIL,
 							"cannot extract ed25519 raw key: %s",
 							ERR_error_string(ERR_get_error(), NULL));
-				EVP_PKEY_free(nkey->specific.key_ssl.key_evp);
-				rspamd_dkim_sign_key_free(nkey);
+				g_free(raw_key);
+				EVP_PKEY_free(evp_temp);
+				BIO_free(bio_temp);
+				g_free(nkey);
 				nkey = NULL;
 				goto end;
 			}
@@ -3415,19 +3420,23 @@ rspamd_dkim_sign_key_load(const char *key, size_t len,
 				/* OpenSSL gives us the 32-byte seed, we need to derive the full key */
 				unsigned char pk[32];
 				unsigned char *full_key = g_malloc(crypto_sign_secretkeybytes());
-				crypto_sign_ed25519_seed_keypair(pk, full_key, nkey->specific.key_eddsa);
-				rspamd_explicit_memzero(nkey->specific.key_eddsa, extracted_len);
-				g_free(nkey->specific.key_eddsa);
-				nkey->specific.key_eddsa = full_key;
-			}
-
-			/* Clean up the EVP_PKEY and BIO as we have the raw key now */
-			EVP_PKEY_free(nkey->specific.key_ssl.key_evp);
-			BIO_free(nkey->specific.key_ssl.key_bio);
-			/* Note: we don't zero out the pointers because 'specific' is a union,
-			 * and zeroing key_ssl would overwrite key_eddsa. The cleanup function
-			 * checks the key type and won't touch key_ssl for EDDSA keys. */
+				crypto_sign_ed25519_seed_keypair(pk, full_key, raw_key);
+				rspamd_explicit_memzero(raw_key, extracted_len);
+				g_free(raw_key);
+				raw_key = full_key;
+				raw_key_len = crypto_sign_secretkeybytes();
+			}
+
+			/* Clean up the EVP_PKEY and BIO before overwriting the union */
+			EVP_PKEY_free(evp_temp);
+			BIO_free(bio_temp);
+
+			/* Now it's safe to assign to the union */
+			nkey->type = RSPAMD_DKIM_KEY_EDDSA;
+			nkey->keylen = raw_key_len;
+			nkey->specific.key_eddsa = raw_key;
 			break;
+		}
 #endif /* HAVE_ED25519 */
 		default: {
 			const char *key_type_str = OBJ_nid2sn(key_type);