The printf format string component %n is a nearly turning-complete gadget.

Largely considered attack surface nowadays.  The benefit provided by %n
is completely overshadowed by the risk.

New uses of %n don't seem to be entering the C ecosystem, as static
tools flag them.  And everyone points fingers at those people....

The list of programs (and libraries) which use %n is therefore finite
and shrinking.  Most of the %n use comes out of the GNU ecosystem.
jca@ has convinced gnulib to fix their code (so we need to wait for
software including gnulib to make new releases).

A few libraries have moved ahead of us and become more strict.  Some n
longer permit %n (for instance, andriod bionic). Others log the occurance.
Some log and abort if the output location is W|X (MacOS).

Our base tree is clean.  The ports tree contains a handful during
build time, and unknown count (more) during runtime.

We would like to abort programs on any occurance of %n.  Or we could
be like MacOS, aborting for W|X pages (but would need a system call
which can check that condition, and that introduces addressspace
knowledge we don't want attackers to know, and may be a poor tradeoff).

For now, we can syslog, to increase awareness, and involve more people
in the greater community to remove %n uses.

[If %n is at the end, use the *printf return value.  If it occurs in
the middle, split the printf calls into multiples]

Hopefully one day, we can just abort() when %n happens.  Help us get
there?

ok jca, plus naddy for ports team

1 files changed, 41 insertions, 2 deletions

diff --git a/lib/libc/stdio/vfwprintf.c b/lib/libc/stdio/vfwprintf.c
index e28901508fa..cadbed15337 100644
--- a/lib/libc/stdio/vfwprintf.c
+++ b/lib/libc/stdio/vfwprintf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfwprintf.c,v 1.19 2017/11/16 08:16:03 tb Exp $ */
+/*	$OpenBSD: vfwprintf.c,v 1.20 2020/10/27 21:06:57 deraadt Exp $ */
 /*-
  * Copyright (c) 1990 The Regents of the University of California.
  * All rights reserved.
@@ -55,6 +55,9 @@
 #include "local.h"
 #include "fvwrite.h"
 
+static const char n_msg[] = ": *wprintf used %n";
+static int n_report;
+
 union arg {
 	int			intarg;
 	unsigned int		uintarg;
@@ -783,7 +786,42 @@ fp_common:
 			break;
 #endif /* FLOATING_POINT */
 #ifndef NO_PRINTF_PERCENT_N
-		case 'n':
+		case 'n': {
+			int maxprot = 0;
+
+#ifdef notyet
+			if (mprotections(fmt0, wcslen(fmt0) + 1,
+			    &maxprot) == -1)
+				maxprot = 0;
+			maxprot &= ~PROT_READ;
+#endif
+
+			if (n_report == 0 || maxprot) {
+				char buf[1024], *p;
+
+				/* <10> is LOG_CRIT */
+				strlcpy(buf, "<10>", sizeof buf);
+
+				/* XXX */
+				if ((p = getenv("_THIS_PORT")) != NULL) {
+					strlcat(buf, p, sizeof buf);
+					strlcat(buf, " ", sizeof buf);
+				}
+
+				/* Make sure progname does not fill the whole buffer */
+				strlcat(buf, __progname, sizeof(buf) - sizeof n_msg);
+				strlcat(buf, n_msg, sizeof buf);
+				/* XXX should we include the wide string? */
+				if ((p = strchr(buf, '\n')))
+					*p = '\0';
+				if (maxprot)
+					strlcat(buf, ": aborting", sizeof buf);
+				sendsyslog(buf, strlen(buf), LOG_CONS);
+				n_report = 1;
+			}
+			if (maxprot)
+				abort();
+
 			if (flags & LLONGINT)
 				*GETARG(long long *) = ret;
 			else if (flags & LONGINT)
@@ -801,6 +839,7 @@ fp_common:
 			else
 				*GETARG(int *) = ret;
 			continue;	/* no output */
+			}
 #endif /* NO_PRINTF_PERCENT_N */
 		case 'O':
 			flags |= LONGINT;