Skip to content

Commit b1230ad

Browse files
besser82besser82
committed
crypt-(gensalt-)static: Do not overwrite the output buffer too early.
Allow passing a pointer returned by crypt(3) previously as the input for a subsequent call to crypt(3), adapt the manpage accordingly, and implement a simple testcase for such a scenario. Also apply the same semantics to crypt_gensalt(3). Fixes #209.
1 parent e34e063 commit b1230ad

File tree

8 files changed

+263
-4
lines changed

8 files changed

+263
-4
lines changed

‎.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ test/crypt-des
7272
test/crypt-gost-yescrypt
7373
test/crypt-kat
7474
test/crypt-md5
75+
test/crypt-nested-call
7576
test/crypt-nthash
7677
test/crypt-pbkdf1-sha1
7778
test/crypt-scrypt
@@ -86,6 +87,7 @@ test/explicit-bzero
8687
test/fcrypt-enosys
8788
test/gensalt
8889
test/gensalt-bcrypt_x
90+
test/gensalt-nested-call
8991
test/gensalt-nthash
9092
test/gensalt-extradata
9193
test/getrandom-fallbacks

‎Makefile.am

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,11 +385,13 @@ check_PROGRAMS = \
385385
test/compile-strong-alias \
386386
test/crypt-badargs \
387387
test/crypt-gost-yescrypt \
388+
test/crypt-nested-call \
388389
test/crypt-sm3-yescrypt \
389390
test/explicit-bzero \
390391
test/gensalt \
391392
test/gensalt-bcrypt_x \
392393
test/gensalt-extradata \
394+
test/gensalt-nested-call \
393395
test/gensalt-nthash \
394396
test/getrandom-fallbacks \
395397
test/getrandom-interface \
@@ -499,12 +501,14 @@ COMMON_TEST_OBJECTS = libcrypt.la
499501
test_badsalt_LDADD = $(COMMON_TEST_OBJECTS)
500502
test_badsetting_LDADD = $(COMMON_TEST_OBJECTS)
501503
test_gensalt_bcrypt_x_LDADD = $(COMMON_TEST_OBJECTS)
504+
test_gensalt_nested_call_LDADD = $(COMMON_TEST_OBJECTS)
502505
test_gensalt_nthash_LDADD = $(COMMON_TEST_OBJECTS)
503506
test_gensalt_extradata_LDADD = $(COMMON_TEST_OBJECTS)
504507
test_checksalt_LDADD = $(COMMON_TEST_OBJECTS)
505508
test_des_obsolete_LDADD = $(COMMON_TEST_OBJECTS)
506509
test_des_obsolete_r_LDADD = $(COMMON_TEST_OBJECTS)
507510
test_crypt_badargs_LDADD = $(COMMON_TEST_OBJECTS)
511+
test_crypt_nested_call_LDADD = $(COMMON_TEST_OBJECTS)
508512
test_short_outbuf_LDADD = $(COMMON_TEST_OBJECTS)
509513
test_preferred_method_LDADD = $(COMMON_TEST_OBJECTS)
510514
test_special_char_salt_LDADD = $(COMMON_TEST_OBJECTS)

‎doc/crypt.3

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,12 @@ which will be overwritten by subsequent calls to
221221
It is not safe to call
222222
.Nm crypt
223223
from multiple threads simultaneously.
224+
It's also not recommended to use the pointer
225+
returned as an argument for another call to
226+
.Nm crypt ,
227+
as some implementations, including earlier
228+
releases of libxcrypt, may overwrite the underlying
229+
static output buffer before computing the hash.
224230
.Pp
225231
.Nm crypt_r ,
226232
.Nm crypt_rn ,

‎doc/crypt_gensalt.3

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ which will be overwritten by subsequent calls to
135135
It is not safe to call
136136
.Nm crypt_gensalt
137137
from multiple threads simultaneously.
138+
It's also not recommended to use the pointer
139+
returned as an argument for another call to
140+
.Nm crypt_gensalt ,
141+
as some implementations, including earlier
142+
releases of libxcrypt, may overwrite the underlying
143+
static output buffer before computing the hash.
138144
However, it
139145
.Em is
140146
safe to pass the string returned by

‎lib/crypt-gensalt-static.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* Copyright (C) 2007-2017 Thorsten Kukuk
2+
Copyright (C) 2025 Björn Esser
23
34
This library is free software; you can redistribute it and/or
45
modify it under the terms of the GNU Lesser General Public License
@@ -26,9 +27,14 @@ crypt_gensalt (const char *prefix, unsigned long count,
2627
const char *rbytes, int nrbytes)
2728
{
2829
static char output[CRYPT_GENSALT_OUTPUT_SIZE];
30+
char buf[CRYPT_GENSALT_OUTPUT_SIZE];
2931

30-
return crypt_gensalt_rn (prefix, count,
31-
rbytes, nrbytes, output, sizeof (output));
32+
crypt_gensalt_rn (prefix, count,
33+
rbytes, nrbytes,
34+
buf, sizeof (buf));
35+
strcpy_or_abort (output, sizeof (output), buf);
36+
37+
return output[0] == '*' ? 0 : output;
3238
}
3339
SYMVER_crypt_gensalt;
3440
#endif

‎lib/crypt-static.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* Copyright (C) 2007-2017 Thorsten Kukuk
2-
Copyright (C) 2019 Björn Esser
2+
Copyright (C) 2019-2025 Björn Esser
33
44
This library is free software; you can redistribute it and/or
55
modify it under the terms of the GNU Lesser General Public License
@@ -25,8 +25,18 @@
2525
char *
2626
crypt (const char *key, const char *setting)
2727
{
28+
static char output[CRYPT_OUTPUT_SIZE];
2829
static struct crypt_data nr_crypt_ctx;
29-
return crypt_r (key, setting, &nr_crypt_ctx);
30+
31+
crypt_r (key, setting, &nr_crypt_ctx);
32+
strcpy_or_abort (output, sizeof (output), nr_crypt_ctx.output);
33+
explicit_bzero (nr_crypt_ctx.output, sizeof (nr_crypt_ctx.output));
34+
35+
#if ENABLE_FAILURE_TOKENS
36+
return output;
37+
#else
38+
return output[0] == '*' ? 0 : output;
39+
#endif /* ENABLE_FAILURE_TOKENS */
3040
}
3141
SYMVER_crypt;
3242
#endif

‎test/crypt-nested-call.c

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright (c) 2025 Björn Esser <besser82 at fedoraproject.org>
3+
* All rights reserved.
4+
*
5+
* Permission to use, copy, modify, and/or distribute this software for any
6+
* purpose with or without fee is hereby granted.
7+
*
8+
* THE SOFTWARE IS PROVIDED “AS IS” AND THE AUTHOR DISCLAIMS ALL WARRANTIES
9+
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
10+
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
11+
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
12+
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
13+
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
14+
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
15+
*/
16+
17+
#include "crypt-port.h"
18+
#include <stdlib.h>
19+
#include <stdio.h>
20+
21+
#define PASSW "alexander"
22+
23+
static const char *settings[] =
24+
{
25+
#if INCLUDE_descrypt
26+
"Mp",
27+
#endif
28+
#if INCLUDE_bigcrypt
29+
"Mp............",
30+
#endif
31+
#if INCLUDE_bsdicrypt
32+
"_J9..MJHn",
33+
#endif
34+
#if INCLUDE_md5crypt
35+
"$1$MJHnaAke",
36+
#endif
37+
#if INCLUDE_nt
38+
"$3$",
39+
#endif
40+
#if INCLUDE_sunmd5
41+
"$md5$BPm.fm03$",
42+
#endif
43+
#if INCLUDE_sm3crypt
44+
"$sm3$MJHnaAkegEVYHsFK",
45+
#endif
46+
#if INCLUDE_sha1crypt
47+
"$sha1$248488$ggu.H673kaZ5$",
48+
#endif
49+
#if INCLUDE_sha256crypt
50+
"$5$MJHnaAkegEVYHsFK",
51+
#endif
52+
#if INCLUDE_sha512crypt
53+
"$6$MJHnaAkegEVYHsFK",
54+
#endif
55+
#if INCLUDE_bcrypt_a
56+
"$2a$05$UBVLHeMpJ/QQCv3XqJx8zO",
57+
#endif
58+
#if INCLUDE_bcrypt
59+
"$2b$05$UBVLHeMpJ/QQCv3XqJx8zO",
60+
#endif
61+
#if INCLUDE_bcrypt_y
62+
"$2y$05$UBVLHeMpJ/QQCv3XqJx8zO",
63+
#endif
64+
#if INCLUDE_bcrypt_x
65+
"$2x$05$UBVLHeMpJ/QQCv3XqJx8zO",
66+
#endif
67+
#if INCLUDE_yescrypt
68+
"$y$j9T$MJHnaAkegEVYHsFKkmfzJ1",
69+
#endif
70+
#if INCLUDE_scrypt
71+
"$7$CU..../....MJHnaAkegEVYHsFKkmfzJ1",
72+
#endif
73+
#if INCLUDE_gost_yescrypt
74+
"$gy$j9T$MJHnaAkegEVYHsFKkmfzJ1",
75+
#endif
76+
#if INCLUDE_sm3_yescrypt
77+
"$sm3y$j9T$MJHnaAkegEVYHsFKkmfzJ1",
78+
#endif
79+
};
80+
81+
int
82+
main (void)
83+
{
84+
char *retval = NULL;
85+
int status = 0;
86+
87+
for (size_t i = 0; i < ARRAY_SIZE (settings); i++)
88+
{
89+
retval = crypt (PASSW, settings[i]);
90+
retval = crypt (PASSW, retval);
91+
92+
if (!retval || *retval == '*')
93+
{
94+
printf ("Subsequent call to crypt(3) with output as setting "
95+
"failed for prefix \"%s\".\n",
96+
settings[i]);
97+
status = 1;
98+
}
99+
100+
retval = crypt (retval, settings[i]);
101+
102+
if (!retval || *retval == '*')
103+
{
104+
printf ("Subsequent call to crypt(3) with output as key "
105+
"failed for prefix \"%s\".\n",
106+
settings[i]);
107+
status = 1;
108+
}
109+
}
110+
111+
return status;
112+
}

‎test/gensalt-nested-call.c

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
* Copyright (c) 2025 Björn Esser <besser82 at fedoraproject.org>
3+
* All rights reserved.
4+
*
5+
* Permission to use, copy, modify, and/or distribute this software for any
6+
* purpose with or without fee is hereby granted.
7+
*
8+
* THE SOFTWARE IS PROVIDED “AS IS” AND THE AUTHOR DISCLAIMS ALL WARRANTIES
9+
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
10+
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
11+
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
12+
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
13+
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
14+
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
15+
*/
16+
17+
#include "crypt-port.h"
18+
19+
#if INCLUDE_bcrypt || INCLUDE_bcrypt_a || INCLUDE_bcrypt_y || \
20+
INCLUDE_bigcrypt || INCLUDE_bsdicrypt || INCLUDE_descrypt || \
21+
INCLUDE_gost_yescrypt || INCLUDE_md5crypt || INCLUDE_nt || \
22+
INCLUDE_scrypt || INCLUDE_sha1crypt || INCLUDE_sha256crypt || \
23+
INCLUDE_sha512crypt || INCLUDE_sm3_yescrypt || INCLUDE_sm3crypt || \
24+
INCLUDE_sunmd5 || INCLUDE_yescrypt
25+
26+
#include <stdio.h>
27+
28+
static const char *prefixes[] =
29+
{
30+
#if INCLUDE_descrypt
31+
"Mp",
32+
#endif
33+
#if INCLUDE_bigcrypt
34+
"Mp............",
35+
#endif
36+
#if INCLUDE_bsdicrypt
37+
"_",
38+
#endif
39+
#if INCLUDE_md5crypt
40+
"$1$",
41+
#endif
42+
#if INCLUDE_nt
43+
"$3$",
44+
#endif
45+
#if INCLUDE_sunmd5
46+
"$md5$",
47+
#endif
48+
#if INCLUDE_sm3crypt
49+
"$sm3$",
50+
#endif
51+
#if INCLUDE_sha1crypt
52+
"$sha1$",
53+
#endif
54+
#if INCLUDE_sha256crypt
55+
"$5$",
56+
#endif
57+
#if INCLUDE_sha512crypt
58+
"$6$",
59+
#endif
60+
#if INCLUDE_bcrypt_a
61+
"$2a$",
62+
#endif
63+
#if INCLUDE_bcrypt
64+
"$2b$",
65+
#endif
66+
#if INCLUDE_bcrypt_y
67+
"$2y$",
68+
#endif
69+
#if INCLUDE_yescrypt
70+
"$y$",
71+
#endif
72+
#if INCLUDE_scrypt
73+
"$7$",
74+
#endif
75+
#if INCLUDE_gost_yescrypt
76+
"$gy$",
77+
#endif
78+
#if INCLUDE_sm3_yescrypt
79+
"$sm3y$",
80+
#endif
81+
};
82+
83+
int
84+
main (void)
85+
{
86+
char *retval = NULL;
87+
int status = 0;
88+
89+
for (size_t i = 0; i < ARRAY_SIZE (prefixes); i++)
90+
{
91+
retval = crypt_gensalt (prefixes[i], 0, NULL, 0);
92+
retval = !retval ? 0 : crypt_gensalt (retval, 0, NULL, 0);
93+
94+
if (!retval)
95+
{
96+
printf ("Subsequent call to crypt_gensalt(3) failed for prefix \"%s\".\n",
97+
prefixes[i]);
98+
status = 1;
99+
}
100+
}
101+
102+
return status;
103+
}
104+
105+
#else
106+
107+
int
108+
main (void)
109+
{
110+
return 77; /* UNSUPPORTED */
111+
}
112+
113+
#endif /* all, but bcrypt_x only */

0 commit comments

Comments
 (0)