From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 14783 invoked from network); 11 Oct 2021 13:35:53 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 11 Oct 2021 13:35:53 -0000 Received: (qmail 30145 invoked by uid 550); 11 Oct 2021 13:35:48 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 24140 invoked from network); 11 Oct 2021 13:32:27 -0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=eteau4a26zmxk2ijau6sgb3me4huyrhc; d=openwall.com.au; t=1633959123; h=Date:From:To:Subject:Message-ID:MIME-Version:Content-Type; bh=AQkN9WAQ5CVoKLyExo3OHJhmWNfCyToJVazeZav23sI=; b=erPush1j8q0hgpzBFHewR/RwQPO/h9SrH8ryY+oAcutdVK1l6dn4JVtAXZLKDZiW Ozwadqms6/5CMCg6BS7vC9ImvfNA/qJxyAkmGlt7S/tFYRlUpj9Cn6Olfru3gxjZWyO G0ANq4NCZQVh2ASI1lIAOtPXxAOmi7z7VLhkOheg= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=ug7nbtf4gccmlpwj322ax3p6ow6yfsug; d=amazonses.com; t=1633959123; h=Date:From:To:Subject:Message-ID:MIME-Version:Content-Type:Feedback-ID; bh=AQkN9WAQ5CVoKLyExo3OHJhmWNfCyToJVazeZav23sI=; b=JG6CGu3YrCXfZFNOw69zqu28qsruUJREqUpENgqm7OZX6VQGckZN4jyDBIi+7a9U YuDVmQJ61AVN+jXTr3aESEXTjV+bFVTlQLtwOu8MCGRUUiZJiXL+dl9Gee5+kSI1M0/ hM4VPhAHA3F0zB6jO5P3u8zZ/UFJ+E17K+NbefeI= ARC-Seal:i=1; a=rsa-sha256; d=openwall.com.au; s=20180402; t=1633959122; cv=none; b=sZdlOG0IAaAGakotxh//GUsBFRVE7u5njlxTesar0SimlR71sSsFd+XXod7f1i1U2nyqzTC4nLyZvy8YzkfXycjbfl7UPqWl/G42DgrepgkBD0FbC9D3ZN4SxlIjq2AHfuPdnTfEjcXgwY9LiZUa6GVAjk8S/0iJf9vgaZEb1Dxyu3RCVw00tjmIyzqGCLX7Gw1LmY+/upg/lPAmZEernGUgfdyS9UA9u/hlFahqFzfnssSfRO6EJsdpvGe//sASZ7omIVmWw9/6eftq4iFWvl6IbI89XlOx4g41FtzCeiwFfsWRR7QBkbafZN7pyzxB3OhPpjbshOuHMnn6KbEKYQ== ARC-Message-Signature:i=1; a=rsa-sha256; d=openwall.com.au; s=20180402; t=1633959122; c=relaxed/simple; bh=AQkN9WAQ5CVoKLyExo3OHJhmWNfCyToJVazeZav23sI=; h=From:To:Subject; b=jQ/GTTdUJE/5lrMLt+l1NUmmPOCYlZq4bdIL7mGbOcKnHX/swFIqi+bQ96BBLMUHvl3Eh2B4OLNu9iGZeUUEInJkmvo1lcMc97FMbprBVv0CNLXTaLoM+alkkjUS4pw2Np9PpRH7YtAPh4UtO53MPAOJL55A5vkPsdCI6txyFjr5kCueUCt0GBcQFXpJ477yAbU9ei5Ty2SDWIx1RP6U0d5t6OxrmVIqQIFgUD1+CtCimCxeiRexxEOR9JIl+qSE/LJf/XaDrXOti0G7yIuZPyH9FAWKKgSoeFjLlpcwYhuT6A8zw6tp1DwNwLxKyQaCFbRmsC1hY2O1vkU2QDmVdA== ARC-Authentication-Results:i=1; mail.openwall.com.au; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=openwall.com.au; s=20180402; t=1633959122; bh=AQkN9WAQ5CVoKLyExo3OHJhmWNfCyToJVazeZav23sI=; h=From:To:Subject:From; b=aKaNLKVac0CueNpTy1rXrnAXQXWjzDXgubX+8eBW7RbrMjGiOVgZKSPXnNAHL6TfM SPBMVEOxPwKSnr+cxtkTOFpHaFm/0dzohtuWuX0rSwbZmIuu1wPlCLPhu4xEWNb1/k AnR7D3b7rHrZPTnWzpr7Z5fbvjCmXbsqLEvnnVIVoWNjJE/powOkA16ksmzEhwvrZj b+gaeLVde+mkBQ08Adc1fvDI7awlV0NDy7l+b4k6uVdrjnSzTkXkQMrQKdBGqYGT86 yRVQWploHBnnef4mcJ97Zb3qhETgtaoaF+DccfsDKvbsBavd2OAjCByXbsfD74/n+y CRY9IkTvIPbZw== Date: Mon, 11 Oct 2021 13:32:03 +0000 From: "(GalaxyMaster)" To: musl@lists.openwall.com Message-ID: <0100017c6f8d9905-d09f62b3-323a-4133-bcad-e27204cfd62c-000000@email.amazonses.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Feedback-ID: 1.us-east-1.Br0WpcLm0XzNPYp+t39aA5qSwb/HYCx3zC5wkQY3G2s=:AmazonSES X-SES-Outgoing: 2021.10.11-54.240.8.35 Subject: [musl] get/set*ent functions and real world applications Hello, 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 abuse. 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@musl:~/musl-tests $ cat test-ent.c #include #include #include #include int main() { struct passwd *pw; printf("Reading /etc/passwd:\n"); errno = 0; while ((pw = getpwent())) { printf("%s:%s:%u:%u:%s:%s:%s\n", pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_gecos, pw->pw_dir, pw->pw_shell); } if (errno != 0) return errno; return 0; } galaxy@musl:~/musl-tests $ tail -1 /etc/passwd user:x::1000::/home/user/: galaxy@musl:~/musl-tests $ ./test-ent | tail -1 user:x:0:1000::/home/user/: galaxy@musl:~/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 systems. 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 input. Another example, where musl exhibits a different behaviour to Glibc is parsing of erroneous group entries (below, "group" misses the final ':'): === galaxy@musl:~/musl-tests $ cat test-gent.c #include #include #include #include int main() { struct group *gr; char **p; printf("Reading /etc/group:"); errno = 0; while ((gr = getgrent())) { printf("%s:%s:%u:", 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@musl:~/musl-tests $ tail -2 /etc/group galaxy:x:502:group1,group2,group3 group:x:1000 galaxy@musl:~/musl-tests $ ./test-gent | tail -2 users:x:999: galaxy:x:502:group1,group2,group3 galaxy@musl:~/musl-tests $ === Glibc, on the other hand, fixes the not-so-broken record (since it is only supplemental groups which are missing): === [galaxy@archlinux musl-tests]$ tail -2 /etc/group pkgbuilder:x:1001:linux-is-fun,musl-rulez group:x:1000 [galaxy@archlinux musl-tests]$ ./test-gent | tail -2 pkgbuilder:x:1001:linux-is-fun,musl-rulez group:x:1000: [galaxy@archlinux 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 system. 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() :) -- (GM)