Re: [musl] [PATCH v2] cuserid: support invocation with a NULL pointer argument

[Rich Felker <dalias@libc.org>]

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

On Sun, Mar 22, 2020 at 11:05:57AM -0400, Rich Felker wrote:
> On Sun, Mar 22, 2020 at 10:51:56AM +0100, Sören Tempel wrote:
> > Sören Tempel <soeren@soeren-tempel.net> wrote:
> > > From my personal Alpine Linux packaging perspective: I would prefer
> > > either removing the function entirely or supporting the invocation with
> > > a NULL pointer argument. Currently, apps using this with a NULL pointer
> > > (e.g. cdparanoia) will crash at runtime which makes it more difficult to
> > > identify them. If the function would be removed entirely we would be
> > > able to detect, during compilation, which apps use it and patch them
> > > accordingly.
> > 
> > Ping.
> 
> Thanks. I'm trying to catch up on patches and I just confirmed your
> research, so I think the patch is correct. I'll apply soon.

Sorry for taking so long to get back to this. The function seems to
have been significantly broken on top of not supporting a null
argument. See attached three patches which I plan to push if there are
no stupid bugs in them.

Rich

[-- Attachment #2: 0001-cuserid-don-t-return-truncated-results.patch --]
[-- Type: text/plain, Size: 986 bytes --]

From a75283d777ed1827ed247dbb465818a0ce371c8f Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Sat, 13 Feb 2021 13:54:00 -0500
Subject: [PATCH 1/3] cuserid: don't return truncated results

checking the length also drops the need to pull in snprintf.
---
 src/legacy/cuserid.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c
index fd7832e4..3ff115a1 100644
--- a/src/legacy/cuserid.c
+++ b/src/legacy/cuserid.c
@@ -2,6 +2,7 @@
 #include <pwd.h>
 #include <stdio.h>
 #include <unistd.h>
+#include <string.h>
 
 char *cuserid(char *buf)
 {
@@ -10,7 +11,10 @@ char *cuserid(char *buf)
 	long pwb[256];
 	if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw))
 		return 0;
+	size_t len = strnlen(pw.pw_name, L_cuserid);
+	if (len == L_cuserid)
+		return 0;
 	if (!buf) buf = usridbuf;
-	snprintf(buf, L_cuserid, "%s", pw.pw_name);
+	memcpy(buf, pw.pw_name, len+1);
 	return buf;
 }
-- 
2.21.0


[-- Attachment #3: 0002-fix-misuse-of-getpwuid_r-in-cuserid.patch --]
[-- Type: text/plain, Size: 993 bytes --]

From cc577d0e058b53df5c0fe2ac17890a41d77e94c5 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Sat, 13 Feb 2021 13:59:44 -0500
Subject: [PATCH 2/3] fix misuse of getpwuid_r in cuserid

getpwuid_r can return 0 but without a result in the case where there
was no error but no record exists. in that case cuserid was treating
it as success and copying junk out of pw.pw_name to the output buffer.
---
 src/legacy/cuserid.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c
index 3ff115a1..07866acf 100644
--- a/src/legacy/cuserid.c
+++ b/src/legacy/cuserid.c
@@ -9,7 +9,8 @@ char *cuserid(char *buf)
 	static char usridbuf[L_cuserid];
 	struct passwd pw, *ppw;
 	long pwb[256];
-	if (getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw))
+	getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw);
+	if (!ppw)
 		return 0;
 	size_t len = strnlen(pw.pw_name, L_cuserid);
 	if (len == L_cuserid)
-- 
2.21.0


[-- Attachment #4: 0003-fix-error-return-value-for-cuserid.patch --]
[-- Type: text/plain, Size: 1100 bytes --]

From 49b6df3d9f3645de55607f1ac60095b22661b334 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Sat, 13 Feb 2021 14:03:23 -0500
Subject: [PATCH 3/3] fix error return value for cuserid

the historical function was specified to return an empty string in the
caller-provided buffer, not a null pointer, to indicate error when the
argument is non-null. only when the argument is null should it return
a null pointer on error.
---
 src/legacy/cuserid.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/legacy/cuserid.c b/src/legacy/cuserid.c
index 07866acf..dcaf73d4 100644
--- a/src/legacy/cuserid.c
+++ b/src/legacy/cuserid.c
@@ -9,12 +9,13 @@ char *cuserid(char *buf)
 	static char usridbuf[L_cuserid];
 	struct passwd pw, *ppw;
 	long pwb[256];
+	if (buf) *buf = 0;
 	getpwuid_r(geteuid(), &pw, (void *)pwb, sizeof pwb, &ppw);
 	if (!ppw)
-		return 0;
+		return buf;
 	size_t len = strnlen(pw.pw_name, L_cuserid);
 	if (len == L_cuserid)
-		return 0;
+		return buf;
 	if (!buf) buf = usridbuf;
 	memcpy(buf, pw.pw_name, len+1);
 	return buf;
-- 
2.21.0