bug#50155: don't use the conversion specifier %n in emacsclient

bug#50155: don't use the conversion specifier %n in emacsclient

[Omar Polo]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

Tags: patch

Hello,

The usage of the %n conversion specifier for printf(3) is widely
discouraged (see [0] for instance).  The OpenBSD libc in particular
logs to syslog every time an application tries to use %n.

Now, in this specific case it's used in a completely safe manner, but
avoiding it seems pretty easy.

Another option would be

	char *emacsdirend = strchr(sockname + tmpdirlen + 1, '/');

but I'm not sure it would be cleaner.

Thanks,

Omar Polo

[0]: https://man.bsd.lv/Linux-5.06/printf.3#BUGS



In GNU Emacs 28.0.50 (build 11, x86_64-unknown-openbsd6.9, X toolkit, cairo version 1.16.0, Xaw scroll bars)
 of 2021-08-17 built on venera
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: OpenBSD venera 6.9 GENERIC.MP#158 amd64

Configured using:
 'configure --prefix=/home/op/opt/emacs --with-x-toolkit=lucid CC=cc'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-don-t-use-n.patch --]
[-- Type: text/patch, Size: 1541 bytes --]

From 18585de41f2d0dc442fe2122c25165f07913c974 Mon Sep 17 00:00:00 2001
From: Omar Polo <op@omarpolo.com>
Date: Sat, 21 Aug 2021 21:32:22 +0000
Subject: [PATCH] don't use %n

---
 lib-src/emacsclient.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 8346524a3e..018e81e422 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1401,10 +1401,8 @@ local_sockname (int s, char sockname[socknamesize], int tmpdirlen,
   /* Put the full address name into the buffer, since the caller might
      need it for diagnostics.  But don't overrun the buffer.  */
   uintmax_t uidmax = uid;
-  int emacsdirlen;
   int suffixlen = snprintf (sockname + tmpdirlen, socknamesize - tmpdirlen,
-			    "/emacs%"PRIuMAX"%n/%s", uidmax, &emacsdirlen,
-			    server_name);
+			    "/emacs%"PRIuMAX"/%s", uidmax, server_name);
   if (! (0 <= suffixlen && suffixlen < socknamesize - tmpdirlen))
     return ENAMETOOLONG;
 
@@ -1412,7 +1410,8 @@ local_sockname (int s, char sockname[socknamesize], int tmpdirlen,
      this user's directory and does not let others write to it; this
      fends off some symlink attacks.  To avoid races, keep the parent
      directory open while checking.  */
-  char *emacsdirend = sockname + tmpdirlen + emacsdirlen;
+  char *emacsdirend = sockname + tmpdirlen + suffixlen -
+    strlen(server_name) - 1;
   *emacsdirend = '\0';
   int dir = openat (AT_FDCWD, sockname,
 		    O_PATH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
-- 
2.32.0

bug#50155: don't use the conversion specifier %n in emacsclient

[Lars Ingebrigtsen]

Omar Polo <op@omarpolo.com> writes:

> Now, in this specific case it's used in a completely safe manner, but
> avoiding it seems pretty easy.

Thanks; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no