Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Mon, 11 Oct 2021 13:32:03 +0000
From: "(GalaxyMaster)" <>
Subject: get/set*ent functions and real world applications


I really enjoy the consise language used in musl, however, I think I found one
place where albeit the code is corect and elegant it does not produce secure
outcome. I am talking about getpwent(), getgrent(), and their counterparts:
putpwent() and putgrent().

All these functions behave well, when the input is well defined.  However,
given the nature of the files these functions are working with and the
sensitivity of the information contained within, it is a point of potential

On a Glibc-based system, if an erronneous entry like "user:x::1000::/home/user:/bin/bash"
it will stay that way as an erroneous entry, but will be ignored, hence being harmless.

On musl-based system, on the other hand, that entry would result in a root
account due to the way atou() works in getpwent_a.c:
galaxy@...l:~/musl-tests $ cat test-ent.c 
#include <sys/types.h>
#include <pwd.h>
#include <stdio.h>
#include <errno.h>

int main() {
	struct passwd *pw;
	printf("Reading /etc/passwd:\n");
	errno = 0;
	while ((pw = getpwent())) {
			pw->pw_name, pw->pw_passwd, pw->pw_uid,
			pw->pw_gid, pw->pw_gecos, pw->pw_dir,
	if (errno != 0) return errno;
	return 0;
galaxy@...l:~/musl-tests $ tail -1 /etc/passwd 
galaxy@...l:~/musl-tests $ ./test-ent | tail -1
galaxy@...l:~/musl-tests $

It is the same story with any numeric field in both /etc/passwd and /etc/group,
empty fields magically turn into 0, which has a significant meaning on Un*x

The whole parssing of password and group entires is quite "dumb", in terms that
it always expects perfectly valid input and produces always correct output, but
I would argue that these functions should be a bit smarter and should not make
assumptions about the validity of the input, i.e treat it as untrusted user

Another example, where musl exhibits a different behaviour to Glibc is parsing
of erroneous group entries (below, "group" misses the final ':'):
galaxy@...l:~/musl-tests $ cat test-gent.c
#include <sys/types.h>
#include <grp.h>
#include <stdio.h>
#include <errno.h>

int main() {
	struct group *gr;
	char **p;
	printf("Reading /etc/group:");
	errno = 0;
	while ((gr = getgrent())) {
			gr->gr_name, gr->gr_passwd, gr->gr_gid);
		for (p =gr->gr_mem; *p; p++) {
			printf("%s%c", *p, *(p+1) ? ',' : '\n');
		if (p == gr->gr_mem) printf("\n");
	if (errno != 0) return errno;
	return 0;
galaxy@...l:~/musl-tests $ tail -2 /etc/group 
galaxy@...l:~/musl-tests $ ./test-gent | tail -2
galaxy@...l:~/musl-tests $

Glibc, on the other hand, fixes the not-so-broken record (since it is only
supplemental groups which are missing):
[galaxy@...hlinux musl-tests]$ tail -2 /etc/group
[galaxy@...hlinux musl-tests]$ ./test-gent | tail -2
[galaxy@...hlinux musl-tests]$

With put*() functions the situation a bit better, but still could be improved
to achieve better compatibility.  putpwent() will output '(null)' for any
NULL pointer passed for the string arguments (due to direct call to fprintf()
and the UB for that situation), while Glibc would output an empty string
instead.  Moreover, putpwent() is inconsistent with putgrent() -- the latter
locks the file before writing and unlocks afterwards, while the former is
just going ahead with fprintf().  I know these funnctions are thread unsafe,
but this lack of locking makes putpwent() plainly dangerous on a multiuser

Rich, would it be acceptable to align musl behaviour with Glibc's in this
regard?  I mean, consider missing uid/gid fields to be a critical error, so the
record is dropped, and consider missing supplemental groups (together with the
last separator) to be a minor, fixable error and preserve the record?

This would make (at least my) life a bit easier in porting applications to a
musl-based system, but more imprtantly, it will be less dangerous if an
erroneous record crawls into one of the identity files.

P.S. I also think that my version (in the example above) of going through
supplemental groups looks nicer than the one in putgrent() :)


Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.