merchant

commit ca44fed55ec43230afc1cc1adeb17d8202b10991
parent 9eb5f4c710ed4777a1789cbc710a188e0996152d
Author: Christian Grothoff <christian@grothoff.org>
Date:   Sat, 10 Oct 2026 22:58:25 +0200

ensure mail addresses are not processed as options, prohibit them starting with '-' and guard 'mail' via '--'

Diffstat:
M
src/backend/taler-merchant-httpd_patch-private-reports-REPORT_ID.c
 | 
22
++++++++++++++++++++++
M
src/backend/taler-merchant-httpd_post-private-reports.c
 | 
22
++++++++++++++++++++++
M
src/backend/taler-merchant-report-generator-email
 | 
11
+++++++----
M
src/include/taler/taler_merchant_util.h
 | 
24
++++++++++++++++++++++++
M
src/util/test_validators.c
 | 
101
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
M
src/util/validators.c
 | 
84
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

6 files changed, 260 insertions(+), 4 deletions(-)
diff --git a/src/backend/taler-merchant-httpd_patch-private-reports-REPORT_ID.c b/src/backend/taler-merchant-httpd_patch-private-reports-REPORT_ID.c
@@ -24,6 +24,7 @@
 #include "taler-merchant-httpd_patch-private-reports-REPORT_ID.h"
 #include <taler/taler_json_lib.h>
 #include <taler/taler_dbevents.h>
+#include "taler/taler_merchant_util.h"
 #include "merchant-database/update_report.h"
 #include "merchant-database/event_notify.h"
 
@@ -105,6 +106,27 @@ TMH_private_patch_report (const struct TMH_RequestHandler *rh,
 
   }
 
+  /* The report generator passes both values to the generator binary on
+     the command line, and the mime type into an "Accept:" header of its
+     own request.  A target address that begins with '-' is taken for an
+     option by the delivery tools behind the generator, and a control
+     character in either value would distort the header. */
+  if (! TALER_MERCHANT_report_target_valid (target_address))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                       "target_address");
+  }
+  if (! TALER_MERCHANT_mime_type_valid (mime_type))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                       "mime_type");
+  }
   qs = TALER_MERCHANTDB_update_report (TMH_db,
                                        hc->instance->settings.id,
                                        report_id,
diff --git a/src/backend/taler-merchant-httpd_post-private-reports.c b/src/backend/taler-merchant-httpd_post-private-reports.c
@@ -24,6 +24,7 @@
 #include "taler-merchant-httpd_post-private-reports.h"
 #include <taler/taler_json_lib.h>
 #include <taler/taler_dbevents.h>
+#include "taler/taler_merchant_util.h"
 #include "merchant-database/insert_report.h"
 #include "merchant-database/event_notify.h"
 
@@ -110,6 +111,27 @@ TMH_private_post_reports (const struct TMH_RequestHandler *rh,
                                        "data_source");
 
   }
+  /* The report generator passes both values to the generator binary on
+     the command line, and the mime type into an "Accept:" header of its
+     own request.  A target address that begins with '-' is taken for an
+     option by the delivery tools behind the generator, and a control
+     character in either value would distort the header. */
+  if (! TALER_MERCHANT_report_target_valid (target_address))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                       "target_address");
+  }
+  if (! TALER_MERCHANT_mime_type_valid (mime_type))
+  {
+    GNUNET_break_op (0);
+    return TALER_MHD_reply_with_error (connection,
+                                       MHD_HTTP_BAD_REQUEST,
+                                       TALER_EC_GENERIC_PARAMETER_MALFORMED,
+                                       "mime_type");
+  }
   qs = TALER_MERCHANTDB_insert_report (TMH_db,
                                        hc->instance->settings.id,
                                        program_section,
diff --git a/src/backend/taler-merchant-report-generator-email b/src/backend/taler-merchant-report-generator-email
@@ -80,8 +80,11 @@ then
   exit 1
 fi
 
-# Validate email address format (basic check)
-if ! echo "$TARGET_ADDRESS" | grep -qE '^[^@]+@[^@]+\.[^@]+$';
+# Validate email address format (basic check).  The local part must not
+# start with '-': such an address would be taken for an option by the
+# mail command below.  The "--" there stops option processing as well,
+# but not every mail implementation honours it.
+if ! echo "$TARGET_ADDRESS" | grep -qE '^[^-@][^@]*@[^@]+\.[^@]+$';
 then
   echo "Error: Invalid email address format: $TARGET_ADDRESS" >&2
   exit 1
@@ -105,7 +108,7 @@ MIME_TYPE=$(echo "$MIME_TYPE" | tr '[:upper:]' '[:lower:]')
 case "$MIME_TYPE" in
   text/plain)
     # For plain text, send directly as email body
-    mail -s "$DESCRIPTION" "$TARGET_ADDRESS"
+    mail -s "$DESCRIPTION" -- "$TARGET_ADDRESS"
     ;;
 
   *)
@@ -154,7 +157,7 @@ case "$MIME_TYPE" in
     FILENAME="report.$EXT"
 
     # Use uuencode method (works with traditional mail command)
-    uuencode "$TMPFILE" "$FILENAME" | mail -s "$DESCRIPTION" "$TARGET_ADDRESS"
+    uuencode "$TMPFILE" "$FILENAME" | mail -s "$DESCRIPTION" -- "$TARGET_ADDRESS"
     ;;
 esac
 
diff --git a/src/include/taler/taler_merchant_util.h b/src/include/taler/taler_merchant_util.h
@@ -153,6 +153,30 @@ TALER_MERCHANT_email_valid (const char *email);
 
 
 /**
+ * Check if @a target_address can be used as the target address of a
+ * report: it is passed to the report generator binary on the command
+ * line, which in turn hands it to a delivery tool.
+ *
+ * @param target_address string to check
+ * @return true if @a target_address is safe to use
+ */
+bool
+TALER_MERCHANT_report_target_valid (const char *target_address);
+
+
+/**
+ * Check if @a mime_type is a syntactically valid MIME type,
+ * that is "type/subtype" possibly followed by parameters,
+ * built only from printable US-ASCII characters.
+ *
+ * @param mime_type string to check
+ * @return true if @a mime_type is a valid MIME type
+ */
+bool
+TALER_MERCHANT_mime_type_valid (const char *mime_type);
+
+
+/**
  * Check if @a email is a valid international phone number
  * with "+CC" prefix.
  *
diff --git a/src/util/test_validators.c b/src/util/test_validators.c
@@ -67,6 +67,10 @@ check_email (void)
     {"user@exam ple.com", false},
     {"user@[256.256.256.256]", false},
     {"user@[IPv6:zzzz::1]", false},
+    /* The local part may legally start with '-', but mail(1) would
+       take such an address for an option */
+    {"-user@example.com", false},
+    {"-Xexec@example.com", false},
   };
 
   unsigned int num_tests = sizeof (tests) / sizeof (tests[0]);
@@ -193,6 +197,91 @@ check_phone (void)
 }
 
 
+static bool
+check_mime_type (void)
+{
+  struct
+  {
+    const char *mime_type;
+    bool expected;
+  } tests[] = {
+    /* Valid MIME types */
+    {"text/plain", true},
+    {"application/pdf", true},
+    {"text/csv; charset=utf-8", true},
+    {"application/vnd.api+json", true},
+    {"TEXT/HTML", true},
+
+    /* Invalid MIME types */
+    {"", false},
+    {"text", false},
+    {"text/", false},
+    {"/plain", false},
+    {"text/plain\r\nAccept: text/html", false},
+    {"text/plain\n", false},
+    {"-text/plain", false},
+    {"text plain", false},
+  };
+  unsigned int num_tests = sizeof (tests) / sizeof (tests[0]);
+  unsigned int failed = 0;
+
+  for (unsigned int i = 0; i < num_tests; i++)
+  {
+    bool result = TALER_MERCHANT_mime_type_valid (tests[i].mime_type);
+
+    if (result == tests[i].expected)
+      continue;
+    failed++;
+    fprintf (stderr,
+             "FAIL: \"%s\" -> %s (expected %s)\n",
+             tests[i].mime_type,
+             result ? "VALID" : "INVALID",
+             tests[i].expected ? "VALID" : "INVALID");
+  }
+  return (failed > 0) ? 1 : 0;
+}
+
+
+static bool
+check_report_target (void)
+{
+  struct
+  {
+    const char *target_address;
+    bool expected;
+  } tests[] = {
+    /* Valid target addresses */
+    {"user@example.com", true},
+    {"/var/spool/taler/report.pdf", true},
+    {"a file with spaces", true},
+
+    /* Invalid target addresses */
+    {"", false},
+    {"-Xexec@example.com", false},
+    {"--exec=/bin/sh", false},
+    {"user@example.com\r\nBcc: other@example.com", false},
+    {"user@example.com\n", false},
+  };
+  unsigned int num_tests = sizeof (tests) / sizeof (tests[0]);
+  unsigned int failed = 0;
+
+  for (unsigned int i = 0; i < num_tests; i++)
+  {
+    bool result = TALER_MERCHANT_report_target_valid (tests[i].target_address);
+
+    if (result == tests[i].expected)
+      continue;
+    failed++;
+    fprintf (stderr,
+             "FAIL: \"%s\" -> %s (expected %s)\n",
+             tests[i].target_address,
+             result ? "VALID" : "INVALID",
+             tests[i].expected ? "VALID" : "INVALID");
+  }
+  return (failed > 0) ? 1 : 0;
+}
+
+
 int
 main (void)
 {
@@ -208,5 +297,17 @@ main (void)
              "phone failed\n");
     return 1;
   }
+  if (check_mime_type ())
+  {
+    fprintf (stderr,
+             "mime_type failed\n");
+    return 1;
+  }
+  if (check_report_target ())
+  {
+    fprintf (stderr,
+             "report target failed\n");
+    return 1;
+  }
   return 0;
 }
diff --git a/src/util/validators.c b/src/util/validators.c
@@ -113,6 +113,17 @@ TALER_MERCHANT_email_valid (const char *email)
   if ('\0' == email[0])
     return false;
 
+  /* A local part may legally begin with '-', but such an address is
+     parsed as an option by the command-line tools that deliver the
+     mail (mail(1), sendmail(1)); the report generator hands the
+     address to one of them.  Refuse it here as well, so that the
+     address stored for an instance is safe for any consumer. */
+  if ('-' == email[0])
+  {
+    GNUNET_break_op (0);
+    return false;
+  }
+
   /* Maximum email length per RFC 5321 */
   if (strlen (email) > 254)
     return false;
@@ -132,6 +143,79 @@ TALER_MERCHANT_email_valid (const char *email)
 }
 
 
+bool
+TALER_MERCHANT_report_target_valid (const char *target_address)
+{
+  if ('\0' == target_address[0])
+  {
+    GNUNET_break_op (0);
+    return false;
+  }
+  /* The report generator passes the target address to the generator
+     binary as the argument of its "-t" option; the binary then hands
+     it to a delivery tool (mail(1) for the e-mail generator), where a
+     leading '-' would be taken for an option. */
+  if ('-' == target_address[0])
+  {
+    GNUNET_break_op (0);
+    return false;
+  }
+  /* The generators are shell scripts and the address may also end up
+     in a mail header; allow printable US-ASCII only. */
+  for (const char *p = target_address; '\0' != *p; p++)
+    if ( (((unsigned char) *p) < 32) ||
+         (((unsigned char) *p) > 126) )
+    {
+      GNUNET_break_op (0);
+      return false;
+    }
+  return true;
+}
+
+
+bool
+TALER_MERCHANT_mime_type_valid (const char *mime_type)
+{
+  regex_t regex;
+  bool is_valid;
+  /*
+   * "type/subtype" of RFC 2045, Section 5.1, optionally followed by
+   * parameters.  Both tokens use the restricted character set of the
+   * RFC; the parameters are only required to be printable US-ASCII,
+   * which is what matters for the consumers: the mime type is written
+   * into an "Accept:" request header of the report generator and
+   * passed to the generator binary on the command line.
+   */
+  const char *pattern =
+    "^"
+    "[a-zA-Z0-9][a-zA-Z0-9!#$&^_.+-]*"
+    "/"
+    "[a-zA-Z0-9][a-zA-Z0-9!#$&^_.+-]*"
+    "([ \t]*;[ -~\t]*)?"
+    "$";
+
+  if ('\0' == mime_type[0])
+  {
+    GNUNET_break_op (0);
+    return false;
+  }
+  GNUNET_assert (0 ==
+                 regcomp (&regex,
+                          pattern,
+                          REG_EXTENDED | REG_NOSUB));
+  is_valid = (0 ==
+              regexec (&regex,
+                       mime_type,
+                       0,
+                       NULL,
+                       0));
+  regfree (&regex);
+  if (! is_valid)
+    GNUNET_break_op (0);
+  return is_valid;
+}
+
+
 char *
 TALER_MERCHANT_phone_validate_normalize (const char *phone,
                                          bool allow_letters)