Skip to content

crypt-(gensalt-)static: Do not overwrite the output buffer too early. #210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ test/crypt-des
test/crypt-gost-yescrypt
test/crypt-kat
test/crypt-md5
test/crypt-nested-call
test/crypt-nthash
test/crypt-pbkdf1-sha1
test/crypt-scrypt
Expand All @@ -86,6 +87,7 @@ test/explicit-bzero
test/fcrypt-enosys
test/gensalt
test/gensalt-bcrypt_x
test/gensalt-nested-call
test/gensalt-nthash
test/gensalt-extradata
test/getrandom-fallbacks
Expand Down
4 changes: 4 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,13 @@ check_PROGRAMS = \
test/compile-strong-alias \
test/crypt-badargs \
test/crypt-gost-yescrypt \
test/crypt-nested-call \
test/crypt-sm3-yescrypt \
test/explicit-bzero \
test/gensalt \
test/gensalt-bcrypt_x \
test/gensalt-extradata \
test/gensalt-nested-call \
test/gensalt-nthash \
test/getrandom-fallbacks \
test/getrandom-interface \
Expand Down Expand Up @@ -499,12 +501,14 @@ COMMON_TEST_OBJECTS = libcrypt.la
test_badsalt_LDADD = $(COMMON_TEST_OBJECTS)
test_badsetting_LDADD = $(COMMON_TEST_OBJECTS)
test_gensalt_bcrypt_x_LDADD = $(COMMON_TEST_OBJECTS)
test_gensalt_nested_call_LDADD = $(COMMON_TEST_OBJECTS)
test_gensalt_nthash_LDADD = $(COMMON_TEST_OBJECTS)
test_gensalt_extradata_LDADD = $(COMMON_TEST_OBJECTS)
test_checksalt_LDADD = $(COMMON_TEST_OBJECTS)
test_des_obsolete_LDADD = $(COMMON_TEST_OBJECTS)
test_des_obsolete_r_LDADD = $(COMMON_TEST_OBJECTS)
test_crypt_badargs_LDADD = $(COMMON_TEST_OBJECTS)
test_crypt_nested_call_LDADD = $(COMMON_TEST_OBJECTS)
test_short_outbuf_LDADD = $(COMMON_TEST_OBJECTS)
test_preferred_method_LDADD = $(COMMON_TEST_OBJECTS)
test_special_char_salt_LDADD = $(COMMON_TEST_OBJECTS)
Expand Down
6 changes: 6 additions & 0 deletions doc/crypt.3
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ which will be overwritten by subsequent calls to
It is not safe to call
.Nm crypt
from multiple threads simultaneously.
It's also not recommended to use the pointer
returned as an argument for another call to
.Nm crypt ,
as some implementations, including earlier
releases of libxcrypt, may overwrite the underlying
static output buffer before computing the hash.
.Pp
.Nm crypt_r ,
.Nm crypt_rn ,
Expand Down
6 changes: 6 additions & 0 deletions doc/crypt_gensalt.3
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ which will be overwritten by subsequent calls to
It is not safe to call
.Nm crypt_gensalt
from multiple threads simultaneously.
It's also not recommended to use the pointer
returned as an argument for another call to
.Nm crypt_gensalt ,
as some implementations, including earlier
releases of libxcrypt, may overwrite the underlying
static output buffer before computing the setting.
However, it
.Em is
safe to pass the string returned by
Expand Down
10 changes: 8 additions & 2 deletions lib/crypt-gensalt-static.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* Copyright (C) 2007-2017 Thorsten Kukuk
Copyright (C) 2025 Björn Esser

This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
Expand Down Expand Up @@ -26,9 +27,14 @@ crypt_gensalt (const char *prefix, unsigned long count,
const char *rbytes, int nrbytes)
{
static char output[CRYPT_GENSALT_OUTPUT_SIZE];
char buf[CRYPT_GENSALT_OUTPUT_SIZE];

return crypt_gensalt_rn (prefix, count,
rbytes, nrbytes, output, sizeof (output));
crypt_gensalt_rn (prefix, count,
rbytes, nrbytes,
buf, sizeof (buf));
strcpy_or_abort (output, sizeof (output), buf);

return output[0] == '*' ? 0 : output;
}
SYMVER_crypt_gensalt;
#endif
Expand Down
14 changes: 12 additions & 2 deletions lib/crypt-static.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* Copyright (C) 2007-2017 Thorsten Kukuk
Copyright (C) 2019 Björn Esser
Copyright (C) 2019-2025 Björn Esser

This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
Expand All @@ -25,8 +25,18 @@
char *
crypt (const char *key, const char *setting)
{
static char output[CRYPT_OUTPUT_SIZE];
static struct crypt_data nr_crypt_ctx;
return crypt_r (key, setting, &nr_crypt_ctx);

crypt_r (key, setting, &nr_crypt_ctx);
strcpy_or_abort (output, sizeof (output), nr_crypt_ctx.output);
explicit_bzero (nr_crypt_ctx.output, sizeof (nr_crypt_ctx.output));

#if ENABLE_FAILURE_TOKENS
return output;
#else
return output[0] == '*' ? 0 : output;
#endif /* ENABLE_FAILURE_TOKENS */
}
SYMVER_crypt;
#endif
Expand Down
112 changes: 112 additions & 0 deletions test/crypt-nested-call.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright (c) 2025 Björn Esser <besser82 at fedoraproject.org>
* All rights reserved.
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted.
*
* THE SOFTWARE IS PROVIDED “AS IS” AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

#include "crypt-port.h"
#include <stdlib.h>
#include <stdio.h>

#define PASSW "alexander"

static const char *settings[] =
{
#if INCLUDE_descrypt
"Mp",
#endif
#if INCLUDE_bigcrypt
"Mp............",
#endif
#if INCLUDE_bsdicrypt
"_J9..MJHn",
#endif
#if INCLUDE_md5crypt
"$1$MJHnaAke",
#endif
#if INCLUDE_nt
"$3$",
#endif
#if INCLUDE_sunmd5
"$md5$BPm.fm03$",
#endif
#if INCLUDE_sm3crypt
"$sm3$MJHnaAkegEVYHsFK",
#endif
#if INCLUDE_sha1crypt
"$sha1$248488$ggu.H673kaZ5$",
#endif
#if INCLUDE_sha256crypt
"$5$MJHnaAkegEVYHsFK",
#endif
#if INCLUDE_sha512crypt
"$6$MJHnaAkegEVYHsFK",
#endif
#if INCLUDE_bcrypt_a
"$2a$05$UBVLHeMpJ/QQCv3XqJx8zO",
#endif
#if INCLUDE_bcrypt
"$2b$05$UBVLHeMpJ/QQCv3XqJx8zO",
#endif
#if INCLUDE_bcrypt_y
"$2y$05$UBVLHeMpJ/QQCv3XqJx8zO",
#endif
#if INCLUDE_bcrypt_x
"$2x$05$UBVLHeMpJ/QQCv3XqJx8zO",
#endif
#if INCLUDE_yescrypt
"$y$j9T$MJHnaAkegEVYHsFKkmfzJ1",
#endif
#if INCLUDE_scrypt
"$7$CU..../....MJHnaAkegEVYHsFKkmfzJ1",
#endif
#if INCLUDE_gost_yescrypt
"$gy$j9T$MJHnaAkegEVYHsFKkmfzJ1",
#endif
#if INCLUDE_sm3_yescrypt
"$sm3y$j9T$MJHnaAkegEVYHsFKkmfzJ1",
#endif
};

int
main (void)
{
char *retval = NULL;
int status = 0;

for (size_t i = 0; i < ARRAY_SIZE (settings); i++)
{
retval = crypt (PASSW, settings[i]);
retval = crypt (PASSW, retval);

if (!retval || *retval == '*')
{
printf ("Subsequent call to crypt(3) with output as setting "
"failed for prefix \"%s\".\n",
settings[i]);
status = 1;
}

retval = crypt (retval, settings[i]);

if (!retval || *retval == '*')
{
printf ("Subsequent call to crypt(3) with output as key "
"failed for prefix \"%s\".\n",
settings[i]);
status = 1;
}
}

return status;
}
113 changes: 113 additions & 0 deletions test/gensalt-nested-call.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Copyright (c) 2025 Björn Esser <besser82 at fedoraproject.org>
* All rights reserved.
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted.
*
* THE SOFTWARE IS PROVIDED “AS IS” AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

#include "crypt-port.h"

#if INCLUDE_bcrypt || INCLUDE_bcrypt_a || INCLUDE_bcrypt_y || \
INCLUDE_bigcrypt || INCLUDE_bsdicrypt || INCLUDE_descrypt || \
INCLUDE_gost_yescrypt || INCLUDE_md5crypt || INCLUDE_nt || \
INCLUDE_scrypt || INCLUDE_sha1crypt || INCLUDE_sha256crypt || \
INCLUDE_sha512crypt || INCLUDE_sm3_yescrypt || INCLUDE_sm3crypt || \
INCLUDE_sunmd5 || INCLUDE_yescrypt

#include <stdio.h>

static const char *prefixes[] =
{
#if INCLUDE_descrypt
"Mp",
#endif
#if INCLUDE_bigcrypt
"Mp............",
#endif
#if INCLUDE_bsdicrypt
"_",
#endif
#if INCLUDE_md5crypt
"$1$",
#endif
#if INCLUDE_nt
"$3$",
#endif
#if INCLUDE_sunmd5
"$md5$",
#endif
#if INCLUDE_sm3crypt
"$sm3$",
#endif
#if INCLUDE_sha1crypt
"$sha1$",
#endif
#if INCLUDE_sha256crypt
"$5$",
#endif
#if INCLUDE_sha512crypt
"$6$",
#endif
#if INCLUDE_bcrypt_a
"$2a$",
#endif
#if INCLUDE_bcrypt
"$2b$",
#endif
#if INCLUDE_bcrypt_y
"$2y$",
#endif
#if INCLUDE_yescrypt
"$y$",
#endif
#if INCLUDE_scrypt
"$7$",
#endif
#if INCLUDE_gost_yescrypt
"$gy$",
#endif
#if INCLUDE_sm3_yescrypt
"$sm3y$",
#endif
};

int
main (void)
{
char *retval = NULL;
int status = 0;

for (size_t i = 0; i < ARRAY_SIZE (prefixes); i++)
{
retval = crypt_gensalt (prefixes[i], 0, NULL, 0);
retval = !retval ? 0 : crypt_gensalt (retval, 0, NULL, 0);

if (!retval)
{
printf ("Subsequent call to crypt_gensalt(3) failed for prefix \"%s\".\n",
prefixes[i]);
status = 1;
}
}

return status;
}

#else

int
main (void)
{
return 77; /* UNSUPPORTED */
}

#endif /* all, but bcrypt_x only */