| Message ID | 20230227153732.123634-1-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] crypt: Remove invalid end of page test badsalttest | expand |
The 02/27/2023 12:37, Adhemerval Zanella via Libc-alpha wrote: > The input argument passes an invalid string without a NUL terminator > on crypt settings inputs, which might lead to invalid OOB on strncmp. > > 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). > > Also adapt the code to use libsupport. > > Checked on arm-linux-gnuabihf. Thanks this looks ok. Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> > --- > crypt/badsalttest.c | 52 ++++++++------------------------------------- > 1 file changed, 9 insertions(+), 43 deletions(-) > > diff --git a/crypt/badsalttest.c b/crypt/badsalttest.c > index bc1e5c1442..b8239a695b 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,24 @@ 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 >
diff --git a/crypt/badsalttest.c b/crypt/badsalttest.c index bc1e5c1442..b8239a695b 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,24 @@ 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>