On 9/2/21 10:46 PM, Jeremie Courreges-Anglas wrote:
On Thu, Sep 02 2021, "Theo de Raadt" <[email protected]> wrote:
Jeremie Courreges-Anglas <[email protected]> wrote:



exim apparently uses printf("%n"), which is currently forbidden (libc
calls abort(3)).

I don't want us to fix all the %n uses in the ports tree, but instead
wait for user reports.  Though for some software like exim it makes
sense to help users avoid such a hard crash.

The diff below doesn't pretend to fix all uses of %n in the exim source.
There may be others that can't be flagged by the compiler (support for
that hesn't been committed yet) because of indirections through wrapper
functions.
+--- src/acl.c.orig
++++ src/acl.c
+@@ -2906,10 +2906,12 @@ for (; cb; cb = cb->next)
+
+   HDEBUG(D_acl)
+     {
+-    int lhswidth = 0;
+-    debug_printf_indent("check %s%s %n",
++    uschar buf[256];
++    int lhswidth = snprintf(CS buf, sizeof buf, "check %s%s ",
+       (!conditions[cb->type].is_modifier && cb->u.negated)? "!":"",
+-      conditions[cb->type].name, &lhswidth);
++      conditions[cb->type].name);
++    if (lhswidth == -1) lhswidth = 0;
++    debug_printf_indent("%s");

Doesn't this %s need an argument buf?

Urkh, indeed, thanks.  New diff below.


Index: Makefile
===================================================================
RCS file: /cvs/ports/mail/exim/Makefile,v
retrieving revision 1.136
diff -u -p -r1.136 Makefile
--- Makefile    5 May 2021 15:08:15 -0000       1.136
+++ Makefile    2 Sep 2021 20:43:34 -0000
@@ -8,7 +8,7 @@ DISTNAME =              exim-${VERSION}
  PKGNAME-main =                exim-${VERSION}
  FULLPKGNAME-eximon =  exim-eximon-${VERSION}
  FULLPKGPATH-eximon =  ${PKGPATH},-eximon
-REVISION-main =                1
+REVISION-main =                2
CATEGORIES = mail Index: patches/patch-src_acl_c
===================================================================
RCS file: patches/patch-src_acl_c
diff -N patches/patch-src_acl_c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_acl_c     2 Sep 2021 20:43:34 -0000
@@ -0,0 +1,23 @@
+$OpenBSD$
+
+Don't use printf %n.
+
+Index: src/acl.c
+--- src/acl.c.orig
++++ src/acl.c
+@@ -2906,10 +2906,12 @@ for (; cb; cb = cb->next)
+
+   HDEBUG(D_acl)
+     {
+-    int lhswidth = 0;
+-    debug_printf_indent("check %s%s %n",
++    uschar buf[256];
++    int lhswidth = snprintf(CS buf, sizeof buf, "check %s%s ",
+       (!conditions[cb->type].is_modifier && cb->u.negated)? "!":"",
+-      conditions[cb->type].name, &lhswidth);
++      conditions[cb->type].name);
++    if (lhswidth == -1) lhswidth = 0;
++    debug_printf_indent("%s", buf);
+
+     if (cb->type == ACLC_SET)
+       {
Index: patches/patch-src_transport_c
===================================================================
RCS file: patches/patch-src_transport_c
diff -N patches/patch-src_transport_c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_transport_c       2 Sep 2021 20:43:34 -0000
@@ -0,0 +1,20 @@
+$OpenBSD$
+
+Don't use printf %n.
+
+Index: src/transport.c
+--- src/transport.c.orig
++++ src/transport.c
+@@ -958,10 +958,9 @@ if (!(tctx->options & topt_no_headers))
+
+   if (tctx->options & topt_add_return_path)
+     {
+-    int n;
+     uschar * s = string_sprintf("Return-path: <%.*s>\n%n",
+-                          EXIM_EMAILADDR_MAX, return_path, &n);
+-    if (!write_chunk(tctx, s, n)) goto bad;
++                          EXIM_EMAILADDR_MAX, return_path);
++    if (!write_chunk(tctx, s, strlen(s))) goto bad;
+     }
+
+   /* Add envelope-to: if requested */



I discussed with exim guys and it seems they are quiet reluctant at modifying "correct C code". At least the acl.c one will cause issues as we have seen with the report from naddy@. So I propose to already commit that diff and check further if there are other issues.


Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to