From 2ba885a62ac26b639d397ce43d2e49fc1e975ddd Mon Sep 17 00:00:00 2001
From: GimmeHardware <23285878+jcastle-gh@users.noreply.github.com>
Date: Sat, 15 Mar 2025 00:58:02 -0400
Subject: [PATCH] Mis-used string pointers in opendkim.c and vbr.c

Enhanced warnings in Debian and Fedora build logs expose some string
addressing problems in vbr.s and opendkim.c.

1. In https://bugs.debian.org/1075339 which is about a different
   problem, the build log attached to the bug shows a warning from
   -DFORTIFY_SOURCE:
  vbr.c: In function 'vbr_query'
  vbr.c:1069:43: warning: "unable to start resolver for.." directive output truncated writing 30 bytes into a region of size 8 [-Wformat-truncation=]
   1069 |                                  "unable to start resolver for '%s'",
        |                                   ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
  vbr.c:1081:43: warning: "unable to start query for '" directive output truncated writing 27 bytes into a region of size 8 [-Wformat-truncation=]
   1081 |                                  "unable to start query for '%s'",
        |                                   ~~~~~~~~^~~~~~~~~~~~~~~~~~~
The size of the buffer pointer is mistakenly used instead of the size of
the buffer itself, resulting in a truncated error message.

2.Looking at the Fedora build log for similar issues shows one in
opendkim.c.
  https://kojipkgs.fedoraproject.org//packages/opendkim/2.11.0/0.42.fc43/data/logs/x86_64/build.log
  opendkim.c: In function 'dkimf_add_signrequest':
  opendkim.c:5023:38: warning: the comparison will always evaluate as 'false' for the address of 'mctx_domain' will never be NULL [-Waddress]
   5023 |                     dfc->mctx_domain == NULL)
        |                                      ^~
  opendkim.c:523:25: note: 'mctx_domain' declared here
   523 |         unsigned char   mctx_domain[DKIM_MAXHOSTNAMELEN + 1];
       |                         ^~~~~~~~~~~
The code is checking if mctx_domain is an empty string. 'mctx_domain'
is the string buffer itself, not a malloced pointer to it, so the code
should be looking for "mctx_domain[0] == '\0'". Later code uses the
string assuming it's not empty, so it seems a good idea to fix it
regardless.
---
 libvbr/vbr.c        | 6 ++----
 opendkim/opendkim.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/libvbr/vbr.c b/libvbr/vbr.c
index cb9124d7..025ae280 100644
--- a/libvbr/vbr.c
+++ b/libvbr/vbr.c
@@ -1065,8 +1065,7 @@ vbr_query(VBR *vbr, u_char **res, u_char **cert)
 		    vbr->vbr_dns_service == NULL &&
 		    vbr->vbr_dns_init(&vbr->vbr_dns_service) != 0)
 		{
-			snprintf(vbr->vbr_error, sizeof vbr->vbr_error,
-			         "unable to start resolver for '%s'",
+			vbr_error(vbr, "unable to start resolver for '%s'",
 			         query);
 			return VBR_STAT_DNSERROR;
 		}
@@ -1077,8 +1076,7 @@ vbr_query(VBR *vbr, u_char **res, u_char **cert)
 
 		if (status != VBR_STAT_OK)
 		{
-			snprintf(vbr->vbr_error, sizeof vbr->vbr_error,
-			         "unable to start query for '%s'",
+			vbr_error(vbr, "unable to start query for '%s'",
 			         query);
 			return VBR_STAT_DNSERROR;
 		}
diff --git a/opendkim/opendkim.c b/opendkim/opendkim.c
index 803f37b0..9e9cbb70 100644
--- a/opendkim/opendkim.c
+++ b/opendkim/opendkim.c
@@ -5020,7 +5020,7 @@ dkimf_add_signrequest(struct msgctx *dfc, DKIMF_DB keytable, char *keyname,
 		}
 
 		if (domain[0] == '%' && domain[1] == '\0' &&
-		    dfc->mctx_domain == NULL)
+		    dfc->mctx_domain[0] == '\0')
 		{
 			if (dolog)
 			{