* [PATCH] crypt: Remove invalid end of page test badsalttest
@ 2023-02-27 12:37 Adhemerval Zanella
2023-02-27 12:59 ` Florian Weimer
2023-02-27 13:29 ` Wilco Dijkstra
0 siblings, 2 replies; 3+ messages in thread
From: Adhemerval Zanella @ 2023-02-27 12:37 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra, Szabolcs Nagy, Florian Weimer
The input argument passes an invalid string with a NULL terminator
on crypt settings inputs, which might lead to invalid OOB on strncmp
(where multiple implementation assumes that the input is always a
NULL terminated string).
Also adapt the code to use libsupport.
Checked on arm-linux-gnuabihf.
---
crypt/badsalttest.c | 50 ++++++++-------------------------------------
1 file changed, 9 insertions(+), 41 deletions(-)
diff --git a/crypt/badsalttest.c b/crypt/badsalttest.c
index bc1e5c1442..acf4ed1ad9 100644
--- a/crypt/badsalttest.c
+++ b/crypt/badsalttest.c
@@ -16,11 +16,12 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/mman.h>
+#include <array_length.h>
+#include <stddef.h>
#include <crypt.h>
+#include <support/check.h>
+
static const char *tests[][2] =
{
{ "no salt", "" },
@@ -30,59 +31,26 @@ static const char *tests[][2] =
{ "both chars bad", ":@" },
{ "un$upported algorithm", "$2$" },
{ "unsupported_algorithm", "_1" },
- { "end of page", NULL }
};
static int
do_test (void)
{
- int result = 0;
struct crypt_data cd;
- size_t n = sizeof (tests) / sizeof (*tests);
- size_t pagesize = (size_t) sysconf (_SC_PAGESIZE);
- char *page;
/* Check that crypt won't look at the second character if the first
one is invalid. */
- page = mmap (NULL, pagesize * 2, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANON, -1, 0);
- if (page == MAP_FAILED)
- {
- perror ("mmap");
- n--;
- }
- else
- {
- if (mmap (page + pagesize, pagesize, 0,
- MAP_PRIVATE | MAP_ANON | MAP_FIXED,
- -1, 0) != page + pagesize)
- perror ("mmap 2");
- page[pagesize - 1] = '*';
- tests[n - 1][1] = &page[pagesize - 1];
- }
-
/* Mark cd as initialized before first call to crypt_r. */
cd.initialized = 0;
- for (size_t i = 0; i < n; i++)
+ for (size_t i = 0; i < array_length (tests); i++)
{
- if (crypt (tests[i][0], tests[i][1]))
- {
- result++;
- printf ("%s: crypt returned non-NULL with salt \"%s\"\n",
- tests[i][0], tests[i][1]);
- }
+ TEST_VERIFY (crypt (tests[i][0], tests[i][1]) == NULL);
- if (crypt_r (tests[i][0], tests[i][1], &cd))
- {
- result++;
- printf ("%s: crypt_r returned non-NULL with salt \"%s\"\n",
- tests[i][0], tests[i][1]);
- }
+ TEST_VERIFY (crypt_r (tests[i][0], tests[i][1], &cd) == NULL);
}
- return result;
+ return 0;
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] crypt: Remove invalid end of page test badsalttest
2023-02-27 12:37 [PATCH] crypt: Remove invalid end of page test badsalttest Adhemerval Zanella
@ 2023-02-27 12:59 ` Florian Weimer
2023-02-27 13:29 ` Wilco Dijkstra
1 sibling, 0 replies; 3+ messages in thread
From: Florian Weimer @ 2023-02-27 12:59 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, Wilco Dijkstra, Szabolcs Nagy
* Adhemerval Zanella:
> The input argument passes an invalid string with a NULL terminator
“null terminator” or “NUL terminator”
> /* Check that crypt won't look at the second character if the first
> one is invalid. */
> - page = mmap (NULL, pagesize * 2, PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANON, -1, 0);
> - if (page == MAP_FAILED)
> - {
> - perror ("mmap");
> - n--;
> - }
> - else
> - {
> - if (mmap (page + pagesize, pagesize, 0,
> - MAP_PRIVATE | MAP_ANON | MAP_FIXED,
> - -1, 0) != page + pagesize)
> - perror ("mmap 2");
> - page[pagesize - 1] = '*';
> - tests[n - 1][1] = &page[pagesize - 1];
> - }
> -
> /* Mark cd as initialized before first call to crypt_r. */
> cd.initialized = 0;
Please delete the comment as well.
Thanks,
Florian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] crypt: Remove invalid end of page test badsalttest
2023-02-27 12:37 [PATCH] crypt: Remove invalid end of page test badsalttest Adhemerval Zanella
2023-02-27 12:59 ` Florian Weimer
@ 2023-02-27 13:29 ` Wilco Dijkstra
1 sibling, 0 replies; 3+ messages in thread
From: Wilco Dijkstra @ 2023-02-27 13:29 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha, Szabolcs Nagy, Florian Weimer
Hi Adhemerval,
> The input argument passes an invalid string with a NULL terminator
*without* a NUL terminator
> on crypt settings inputs, which might lead to invalid OOB on strncmp
> (where multiple implementation assumes that the input is always a
> NULL terminated string).
Implementations only assume there is a NUL terminator if the string is shorter
than the specified size, so strings don't need to always be NUL terminated
(stratcliff.c has tests for this).
Cheers,
Wilco
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-27 13:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-27 12:37 [PATCH] crypt: Remove invalid end of page test badsalttest Adhemerval Zanella
2023-02-27 12:59 ` Florian Weimer
2023-02-27 13:29 ` Wilco Dijkstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).