Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241229085833.719968-2-me@runxiyu.org>
Date: Sun, 29 Dec 2024 16:57:45 +0800
From: Runxi Yu <me@...xiyu.org>
To: musl@...ts.openwall.com,
	me@...xiyu.org
Subject: [PATCH 0/1] crypt(3) returns string literal "*", crashing PAM on Alpine

Hi,

Test_User <hax@...xiyu.org> discovered that using passwd(1) from
shadow-utils on Alpine Linux 3.21 with a password longer than 256
character causes a segmentation fault.  I reported it to
<https://gitlab.alpinelinux.org/alpine/aports/-/issues/16784>, and
people in #alpine-linux helped a bit.

It turns out that musl's crypt(3) returns a "*" from a string literal,
which PAM attempts to erase with pam_overwrite_string.

(Irrelevant lines truncated)

musl/src/crypt/crypt_sha512.c
> 	if (!p || q != testbuf || memcmp(testbuf, testhash, sizeof testhash))
> 		return "*";

pam/modules/pass_unix/passverify.c
> 	sp = crypt(password, salt);
> 	if (!sp || strncmp(algoid, sp, strlen(algoid)) != 0) {
> 		pam_syslog(...);
> 		if(sp) {
> 		   pam_overwrite_string(sp);  /* <-- attempt to overwrite musl's string literal */
> 		}
> 		return NULL;
> 	}

https://pubs.opengroup.org/onlinepubs/9799919799/functions/crypt.html:
> The return value of crypt() points to static data that is overwritten
> by each call.
> 
> Upon successful completion, crypt() shall return a pointer to the
> hashed password; the first two bytes of the returned value shall be
> those of the salt argument. Otherwise, it shall return a null pointer
> and set errno to indicate the error.

I think musl's behavior of returning "*" is incorrect. It's rather
reasonable for the caller to attempt to erase whatever is returned by
crypt, and AFAIK there is no specification that allows returning "*" as
a failure return value.


Note: I am not on the mailing list; please CC me in replies.

--
Best regards,

Runxi Yu (they/them)
Year 11, E House
YK Pao School SJ
https://runxiyu.org

Runxi Yu (1):
  crypt: Do not return "*" from read-only memory regions

 src/crypt/crypt_blowfish.c | 4 +++-
 src/crypt/crypt_des.c      | 6 +++++-
 src/crypt/crypt_md5.c      | 9 ++++++---
 src/crypt/crypt_sha256.c   | 9 ++++++---
 src/crypt/crypt_sha512.c   | 9 ++++++---
 5 files changed, 26 insertions(+), 11 deletions(-)

-- 
2.47.1

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.