From patchwork Thu Jun 28 22:14:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joseph Myers X-Patchwork-Id: 28116 Received: (qmail 3472 invoked by alias); 28 Jun 2018 22:14:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 3462 invoked by uid 89); 28 Jun 2018 22:14:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy=7127 X-HELO: relay1.mentorg.com Date: Thu, 28 Jun 2018 22:14:27 +0000 From: Joseph Myers To: Subject: Avoid insecure usage of tmpnam in tests Message-ID: User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Various glibc testcases use tmpnam in ways subject to race conditions (generate a temporary file name, then later open that file without O_EXCL). This patch fixes those tests to use mkstemp - generally a minimal local fix to use mkstemp instead of tmpnam, rather than a larger fix to use other testsuite infrastructure for temporary files. The unchanged use of tmpnam in posix/wordexp-test.c would fail safe in the event of a race (it's generating a name for use with mkdir rather than for a file to be opened for writing). Tested for x86_64. 2018-06-28 Joseph Myers * grp/tst_fgetgrent.c: Include . (main): Use mkstemp instead of tmpnam. * io/test-utime.c (main): Likewise. * posix/annexc.c (macrofile): Change to modifiable array. (get_null_defines): Use mkstemp instead of tmpnam. Do not remove macrofile here. * posix/bug-getopt1.c: Include . (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt2.c: Include . (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt3.c: Include . (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt4.c: Include . (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt5.c: Include . (do_test): Use mkstemp instead of tmpnam. * stdio-common/bug7.c: Include and . (main): Use mkstemp instead of tmpnam. * stdio-common/tst-fdopen.c: Include . (main): Use mkstemp instead of tmpnam. * stdio-common/tst-ungetc.c: Include . (main): use mkstemp instead of tmpnam. * stdlib/isomac.c (macrofile): Change to modifiable array. (get_null_defines): Use mkstemp instead of tmpnam. Do not remove macrofile here. Reviewed-by: Carlos O'Donell diff --git a/grp/tst_fgetgrent.c b/grp/tst_fgetgrent.c index 501ad99..d612445 100644 --- a/grp/tst_fgetgrent.c +++ b/grp/tst_fgetgrent.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors; @@ -99,7 +100,14 @@ test_fgetgrent (const char *filename) int main (int argc, char *argv[]) { - char *file = tmpnam (NULL); + char file[] = "/tmp/tst_fgetgrent.XXXXXX"; + int fd = mkstemp (file); + if (fd == -1) + { + printf ("mkstemp failed: %m\n"); + return 1; + } + close (fd); int i = 0; if (argc > 1) diff --git a/io/test-utime.c b/io/test-utime.c index 2ad0995..0ab778e 100644 --- a/io/test-utime.c +++ b/io/test-utime.c @@ -27,23 +27,17 @@ int main (int argc, char *argv[]) { - char file[L_tmpnam]; + char file[] = "/tmp/test-utime.XXXXXX"; struct utimbuf ut; struct stat st; struct stat stnow; time_t now1, now2; int fd; - if (tmpnam (file) == 0) - { - perror ("tmpnam"); - return 1; - } - - fd = creat (file, 0666); + fd = mkstemp (file); if (fd < 0) { - perror ("creat"); + perror ("mkstemp"); return 1; } close (fd); diff --git a/posix/annexc.c b/posix/annexc.c index fe3a600..d870441 100644 --- a/posix/annexc.c +++ b/posix/annexc.c @@ -26,7 +26,7 @@ #define HEADER_MAX 256 -static const char *macrofile; +static char macrofile[] = "/tmp/annexc.XXXXXX"; /* . */ static const char *const aio_syms[] = @@ -712,7 +712,13 @@ get_null_defines (void) FILE *input; int first = 1; - macrofile = tmpnam (NULL); + int fd = mkstemp (macrofile); + if (fd == -1) + { + printf ("mkstemp failed: %m\n"); + exit (1); + } + close (fd); command = malloc (sizeof fmt + sizeof "/dev/null" + 2 * strlen (CC) + strlen (INC) + strlen (macrofile)); @@ -784,7 +790,6 @@ get_null_defines (void) } result[result_len] = NULL; fclose (input); - remove (macrofile); return (const char **) result; } diff --git a/posix/bug-getopt1.c b/posix/bug-getopt1.c index a47dc7e..a5a3711 100644 --- a/posix/bug-getopt1.c +++ b/posix/bug-getopt1.c @@ -1,6 +1,7 @@ /* BZ 11039 */ #include #include +#include static int one_test (const char *fmt, int argc, char *argv[], int expected[argc - 1]) @@ -39,12 +40,14 @@ one_test (const char *fmt, int argc, char *argv[], int expected[argc - 1]) static int do_test (void) { - char *fname = tmpnam (NULL); - if (fname == NULL) + char fname[] = "/tmp/bug-getopt1.XXXXXX"; + int fd = mkstemp (fname); + if (fd == -1) { - puts ("cannot generate name for temporary file"); + printf ("mkstemp failed: %m\n"); return 1; } + close (fd); if (freopen (fname, "w+", stderr) == NULL) { diff --git a/posix/bug-getopt2.c b/posix/bug-getopt2.c index 93c3035..8f92f0c 100644 --- a/posix/bug-getopt2.c +++ b/posix/bug-getopt2.c @@ -1,6 +1,7 @@ /* BZ 11039 */ #include #include +#include static int one_test (const char *fmt, int argc, char *argv[], int expected[argc - 1]) @@ -37,12 +38,14 @@ one_test (const char *fmt, int argc, char *argv[], int expected[argc - 1]) static int do_test (void) { - char *fname = tmpnam (NULL); - if (fname == NULL) + char fname[] = "/tmp/bug-getopt2.XXXXXX"; + int fd = mkstemp (fname); + if (fd == -1) { - puts ("cannot generate name for temporary file"); + printf ("mkstemp failed: %m\n"); return 1; } + close (fd); if (freopen (fname, "w+", stderr) == NULL) { diff --git a/posix/bug-getopt3.c b/posix/bug-getopt3.c index c3a8cb2..45a8d3e 100644 --- a/posix/bug-getopt3.c +++ b/posix/bug-getopt3.c @@ -2,6 +2,7 @@ #include #include #include +#include static const struct option opts[] = { @@ -48,12 +49,14 @@ one_test (const char *fmt, int argc, char *argv[], int n, int expected[n], static int do_test (void) { - char *fname = tmpnam (NULL); - if (fname == NULL) + char fname[] = "/tmp/bug-getopt3.XXXXXX"; + int fd = mkstemp (fname); + if (fd == -1) { - puts ("cannot generate name for temporary file"); + printf ("mkstemp failed: %m\n"); return 1; } + close (fd); if (freopen (fname, "w+", stderr) == NULL) { diff --git a/posix/bug-getopt4.c b/posix/bug-getopt4.c index 0956ca5..c5e3c14 100644 --- a/posix/bug-getopt4.c +++ b/posix/bug-getopt4.c @@ -2,6 +2,7 @@ #include #include #include +#include static const struct option opts[] = { @@ -52,12 +53,14 @@ one_test (const char *fmt, int argc, char *argv[], int n, int expected[n]) static int do_test (void) { - char *fname = tmpnam (NULL); - if (fname == NULL) + char fname[] = "/tmp/bug-getopt4.XXXXXX"; + int fd = mkstemp (fname); + if (fd == -1) { - puts ("cannot generate name for temporary file"); + printf ("mkstemp failed: %m\n"); return 1; } + close (fd); if (freopen (fname, "w+", stderr) == NULL) { diff --git a/posix/bug-getopt5.c b/posix/bug-getopt5.c index ed2639d..4f67d9b 100644 --- a/posix/bug-getopt5.c +++ b/posix/bug-getopt5.c @@ -2,6 +2,7 @@ #include #include #include +#include static const struct option opts[] = { @@ -47,12 +48,14 @@ one_test (const char *fmt, int argc, char *argv[], int n, int expected[n]) static int do_test (void) { - char *fname = tmpnam (NULL); - if (fname == NULL) + char fname[] = "/tmp/bug-getopt5.XXXXXX"; + int fd = mkstemp (fname); + if (fd == -1) { - puts ("cannot generate name for temporary file"); + printf ("mkstemp failed: %m\n"); return 1; } + close (fd); if (freopen (fname, "w+", stderr) == NULL) { diff --git a/stdio-common/bug7.c b/stdio-common/bug7.c index 2b1efe3..c9c2ef5 100644 --- a/stdio-common/bug7.c +++ b/stdio-common/bug7.c @@ -1,21 +1,25 @@ /* Regression test for fseek and freopen bugs. */ #include +#include +#include int main (int argc, char *argv[]) { int lose = 0; - char filename[L_tmpnam]; + char filename[] = "/tmp/bug7.XXXXXX"; FILE *fp; - if (tmpnam (filename) == NULL) + int fd = mkstemp (filename); + if (fd == -1) { - printf ("tmpnam failed\n"); + printf ("mkstemp failed\n"); lose = 1; } else { + close (fd); fp = fopen (filename, "w+"); fprintf (fp, "Hello world!\n"); fflush (fp); @@ -32,17 +36,21 @@ main (int argc, char *argv[]) { FILE *file1; FILE *file2; - char filename1[L_tmpnam]; - char filename2[L_tmpnam]; + char filename1[] = "/tmp/bug7.XXXXXX"; + char filename2[] = "/tmp/bug7.XXXXXX"; int ch; - if (tmpnam (filename1) == NULL || tmpnam (filename2) == NULL) + int fd1 = mkstemp (filename1); + int fd2 = mkstemp (filename2); + if (fd1 == -1 || fd2 == -1) { - printf ("tmpnam failed\n"); + printf ("mkstemp failed\n"); lose = 1; } else { + close (fd1); + close (fd2); file1 = fopen (filename1, "w"); fclose (file1); diff --git a/stdio-common/tst-fdopen.c b/stdio-common/tst-fdopen.c index e70a0cd..136fff5 100644 --- a/stdio-common/tst-fdopen.c +++ b/stdio-common/tst-fdopen.c @@ -1,6 +1,7 @@ /* Test for fdopen bugs. */ #include +#include #include #include @@ -18,12 +19,18 @@ char buffer[256]; int main (int argc, char *argv[]) { - char *name; + char name[] = "/tmp/tst-fdopen.XXXXXX"; FILE *fp = NULL; int retval = 0; int fd; - name = tmpnam (NULL); + fd = mkstemp (name); + if (fd == -1) + { + printf ("mkstemp failed: %m\n"); + return 1; + } + close (fd); fp = fopen (name, "w"); assert (fp != NULL) fputs ("foobar and baz", fp); diff --git a/stdio-common/tst-ungetc.c b/stdio-common/tst-ungetc.c index 44cf6a6..1344b2b 100644 --- a/stdio-common/tst-ungetc.c +++ b/stdio-common/tst-ungetc.c @@ -1,6 +1,7 @@ /* Test for ungetc bugs. */ #include +#include #include #undef assert @@ -15,13 +16,19 @@ int main (int argc, char *argv[]) { - char *name; + char name[] = "/tmp/tst-ungetc.XXXXXX"; FILE *fp = NULL; int retval = 0; int c; char buffer[64]; - name = tmpnam (NULL); + int fd = mkstemp (name); + if (fd == -1) + { + printf ("mkstemp failed: %m\n"); + return 1; + } + close (fd); fp = fopen (name, "w"); assert (fp != NULL) fputs ("bla", fp); diff --git a/stdlib/isomac.c b/stdlib/isomac.c index 8abf931..235725f 100644 --- a/stdlib/isomac.c +++ b/stdlib/isomac.c @@ -77,7 +77,7 @@ #define HEADER_MAX 256 -static const char *macrofile; +static char macrofile[] = "/tmp/isomac.XXXXXX"; /* ISO C header names including Amendment 1 (without ".h" suffix). */ static char *header[] = @@ -249,7 +249,13 @@ get_null_defines (void) FILE *input; int first = 1; - macrofile = tmpnam (NULL); + int fd = mkstemp (macrofile); + if (fd == -1) + { + printf ("mkstemp failed: %m\n"); + exit (1); + } + close (fd); command = malloc (sizeof fmt + sizeof "/dev/null" + 2 * strlen (CC) + strlen (INC) + strlen (macrofile)); @@ -330,7 +336,6 @@ get_null_defines (void) } result[result_len] = NULL; fclose (input); - remove (macrofile); return (const char **) result; }