Propagate null gstring through string_catn()

8 files changed, 27 insertions, 17 deletions

diff --git a/src/src/arc.c b/src/src/arc.c
index 9678ceb2d..e0ee19950 100644
--- a/src/src/arc.c
+++ b/src/src/arc.c
@@ -1619,7 +1619,7 @@ if (!arc_valid_id(identity))
 if (!arc_valid_id(selector))
   { s = US"selector"; goto bad_arg_ret; }
 if (*privkey == '/' && !(privkey = expand_file_big_buffer(privkey)))
-  return sigheaders ? sigheaders : string_get(0);
+  goto ret_sigheaders;
 
 if ((opts = string_nextinlist(&signspec, &sep, NULL, 0)))
   {
@@ -1678,7 +1678,7 @@ if ((rheaders = arc_sign_scan_headers(&arc_sign_ctx, sigheaders)))
 if (!(arc_sign_find_ar(headers, identity, &ar)))
   {
   log_write(0, LOG_MAIN, "ARC: no Authentication-Results header for signing");
-  return sigheaders ? sigheaders : string_get(0);
+  goto ret_sigheaders;
   }
 
 /* We previously built the data-struct for the existing ARC chain, if any, using a headers
@@ -1734,14 +1734,19 @@ if (g)
 /* Finally, append the dkim headers and return the lot. */
 
 if (sigheaders) g = string_catn(g, sigheaders->s, sigheaders->ptr);
-(void) string_from_gstring(g);
-gstring_release_unused(g);
-return g;
+
+out:
+  if (!g) return string_get(1);
+  (void) string_from_gstring(g);
+  gstring_release_unused(g);
+  return g;
 
 
 bad_arg_ret:
   log_write(0, LOG_MAIN, "ARC: bad signing-specification (%s)", s);
-  return sigheaders ? sigheaders : string_get(0);
+ret_sigheaders:
+  g = sigheaders;
+  goto out;
 }
 
 
diff --git a/src/src/dkim_transport.c b/src/src/dkim_transport.c
index c09c5059b..146faff36 100644
--- a/src/src/dkim_transport.c
+++ b/src/src/dkim_transport.c
@@ -169,7 +169,7 @@ if (!(dkim_signature = dkim_exim_sign(deliver_datafile, SPOOL_DATA_START_OFFSET,
 #ifdef EXPERIMENTAL_ARC
 if (dkim->arc_signspec)			/* Prepend ARC headers */
   {
-  uschar * e;
+  uschar * e = NULL;
   if (!(dkim_signature = arc_sign(dkim->arc_signspec, dkim_signature, &e)))
     {
     *err = e;
diff --git a/src/src/expand.c b/src/src/expand.c
index cd9b48c23..6478920f8 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -4733,7 +4733,7 @@ while (*s)
   skipping, but "break" otherwise so we get debug output for the item
   expansion. */
   {
-  int start = yield->ptr;
+  int start = gstring_length(yield);
   switch(item_type)
     {
     /* Call an ACL from an expansion.  We feed data in via $acl_arg1 - $acl_arg9.
@@ -5956,8 +5956,7 @@ while (*s)
 
         /* Copy the characters before the match, plus the expanded insertion. */
 
-	if (ovec[0] > moffset)
-	  yield = string_catn(yield, subject + moffset, ovec[0] - moffset);
+	yield = string_catn(yield, subject + moffset, ovec[0] - moffset);
 
         if (!(insert = expand_string(sub[2])))
 	  goto EXPAND_FAILED;
@@ -7013,7 +7012,7 @@ while (*s)
     /*NOTREACHED*/
 
   DEBUG(D_expand)
-    if (start > 0 || *s)	/* only if not the sole expansion of the line */
+    if (yield && (start > 0 || *s))	/* only if not the sole expansion of the line */
       debug_expansion_interim(US"item-res",
 			      yield->s + start, yield->ptr - start, skipping);
   continue;
diff --git a/src/src/functions.h b/src/src/functions.h
index ac4ff3b63..39dfc46fe 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -900,7 +900,7 @@ static inline gstring *
 string_get_tainted_trc(unsigned size, BOOL tainted, const char * func, unsigned line)
 {
 gstring * g = store_get_3(sizeof(gstring) + size, tainted, func, line);
-g->size = size;
+g->size = size;		/*XXX would be good if we could see the actual alloc size */
 g->ptr = 0;
 g->s = US(g + 1);
 return g;
diff --git a/src/src/lookups/mysql.c b/src/src/lookups/mysql.c
index 9cec2158b..b36ce0950 100644
--- a/src/src/lookups/mysql.c
+++ b/src/src/lookups/mysql.c
@@ -308,7 +308,7 @@ fields = mysql_fetch_fields(mysql_result);
 
 while ((mysql_row_data = mysql_fetch_row(mysql_result)))
   {
-  unsigned long *lengths = mysql_fetch_lengths(mysql_result);
+  unsigned long * lengths = mysql_fetch_lengths(mysql_result);
 
   if (result)
     result = string_catn(result, US"\n", 1);
@@ -319,7 +319,9 @@ while ((mysql_row_data = mysql_fetch_row(mysql_result)))
 			result);
 
   else if (mysql_row_data[0] != NULL)    /* NULL value yields nothing */
-      result = string_catn(result, US mysql_row_data[0], lengths[0]);
+      result = lengths[0] == 0 && !result
+	? string_get(1)		/* for 0-len string result ensure non-null gstring */
+        : string_catn(result, US mysql_row_data[0], lengths[0]);
   }
 
 /* more results? -1 = no, >0 = error, 0 = yes (keep looping)
diff --git a/src/src/lookups/pgsql.c b/src/src/lookups/pgsql.c
index c121cb668..28d4024d8 100644
--- a/src/src/lookups/pgsql.c
+++ b/src/src/lookups/pgsql.c
@@ -336,6 +336,7 @@ for (int i = 0; i < num_tuples; i++)
       uschar *tmp = US PQgetvalue(pg_result, i, j);
       result = lf_quote(US PQfname(pg_result, j), tmp, Ustrlen(tmp), result);
       }
+  if (!result) result = string_get(1);
   }
 
 /* If result is NULL then no data has been found and so we return FAIL. */
diff --git a/src/src/lookups/sqlite.c b/src/src/lookups/sqlite.c
index 4c1412bae..d8a11ba12 100644
--- a/src/src/lookups/sqlite.c
+++ b/src/src/lookups/sqlite.c
@@ -66,7 +66,7 @@ if (argc > 1)
   /* For multiple fields, include the field name too */
   for (int i = 0; i < argc; i++)
     {
-    uschar *value = US((argv[i] != NULL)? argv[i]:"<NULL>");
+    uschar * value = US(argv[i] ? argv[i] : "<NULL>");
     res = lf_quote(US azColName[i], value, Ustrlen(value), res);
     }
   }
@@ -74,7 +74,8 @@ if (argc > 1)
 else
   res = string_cat(res, argv[0] ? US argv[0] : US "<NULL>");
 
-*(gstring **)arg = res;
+/* always return a non-null gstring, even for a zero-length string result */
+*(gstring **)arg = res ? res : string_get(1);
 return 0;
 }
 
@@ -94,7 +95,7 @@ if (ret != SQLITE_OK)
   return FAIL;
   }
 
-if (!res) *do_cache = 0;
+if (!res) *do_cache = 0;	/* on fail, wipe cache */
 
 *result = string_from_gstring(res);
 return OK;
diff --git a/src/src/string.c b/src/src/string.c
index 12666b734..8ecbc0cd7 100644
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -1139,6 +1139,7 @@ Returns:   growable string, changed if copied for expansion.
            Note that a NUL is not added, though space is left for one. This is
            because string_cat() is often called multiple times to build up a
            string - there's no point adding the NUL till the end.
+	   NULL is a possible return.
 
 */
 /* coverity[+alloc] */
@@ -1152,6 +1153,7 @@ BOOL srctaint = is_tainted(s);
 if (count < 0)
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
     "internal error in string_catn (count %d)", count);
+if (count == 0) return g;
 
 if (!g)
   {