| Message ID | alpine.DEB.2.20.1806282214140.25581@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
| Series | Avoid insecure usage of tmpnam in tests | expand |
Ping. This patch <https://sourceware.org/ml/libc-alpha/2018-06/msg00927.html> is pending review.
On 06/28/2018 06:14 PM, Joseph Myers wrote: > 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). I empathize with your strategy to make this a minimal fix. > Tested for x86_64. OK with explanation of inclusion of unistd.h in tst_fgetgrent.c or removal. OK with explanation of file cleanup in annex.c and isomac.c or addition of remove(). Reviewed-by: Carlos O'Donell <carlos@redhat.com> > 2018-06-28 Joseph Myers <joseph@codesourcery.com> > > * grp/tst_fgetgrent.c: Include <unistd.h>. > (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 <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt2.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt3.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt4.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt5.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * stdio-common/bug7.c: Include <stdlib.h> and <unistd.h>. > (main): Use mkstemp instead of tmpnam. > * stdio-common/tst-fdopen.c: Include <stdlib.h>. > (main): Use mkstemp instead of tmpnam. > * stdio-common/tst-ungetc.c: Include <stdlib.h>. > (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. > > 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 <stdlib.h> > #include <string.h> > #include <sys/types.h> > +#include <unistd.h> Why unistd.h? > > 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); OK. > 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"; OK. > 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); OK. > if (fd < 0) > { > - perror ("creat"); > + perror ("mkstemp"); OK. > 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"; OK. > > /* <aio.h>. */ > 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); Don't we still need to remove the temporary file? > > 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 <unistd.h> > #include <stdio.h> > +#include <stdlib.h> OK. Keeping it minimal (this test is pretty ugly and needs more cleanup). > > 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); OK. Reviewed that fname is removed. > > 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 <unistd.h> > #include <stdio.h> > +#include <stdlib.h> OK. > > 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); OK. Reviewed that fname is removed. > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> OK. > > 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); OK. Reviewed that fname is removed. > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> OK. > > 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); OK. > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> OK. > > 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); OK. Reviewed that fname is removed. > > 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 <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> OK. > > int > main (int argc, char *argv[]) > { > int lose = 0; > - char filename[L_tmpnam]; > + char filename[] = "/tmp/bug7.XXXXXX"; > FILE *fp; > OK. > - if (tmpnam (filename) == NULL) > + int fd = mkstemp (filename); > + if (fd == -1) > { > - printf ("tmpnam failed\n"); > + printf ("mkstemp failed\n"); OK. > lose = 1; > } > else > { > + close (fd); OK. Verified filename is removed. > 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"; OK. > 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"); OK. > lose = 1; > } > else > { > + close (fd1); > + close (fd2); OK. Verified filename1 and filename2 are removed. > > 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 <stdio.h> > +#include <stdlib.h> > #include <unistd.h> > #include <fcntl.h> > OK. > @@ -18,12 +19,18 @@ char buffer[256]; > int > main (int argc, char *argv[]) > { > - char *name; > + char name[] = "/tmp/tst-fdopen.XXXXXX"; OK. > 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); OK. Verified name is unlink'd. > 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 <stdio.h> > +#include <stdlib.h> > #include <unistd.h> OK. > > #undef assert > @@ -15,13 +16,19 @@ > int > main (int argc, char *argv[]) > { > - char *name; > + char name[] = "/tmp/tst-ungetc.XXXXXX"; OK. > 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); OK. Verified name is unlink'd. > 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); OK. > > 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); Same issue as above. > > return (const char **) result; > } >
On Fri, 6 Jul 2018, Carlos O'Donell wrote: > > 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 <stdlib.h> > > #include <string.h> > > #include <sys/types.h> > > +#include <unistd.h> > > Why unistd.h? Because of the call to close that is added. > > @@ -784,7 +790,6 @@ get_null_defines (void) > > } > > result[result_len] = NULL; > > fclose (input); > > - remove (macrofile); > > Don't we still need to remove the temporary file? The same temporary file has a name generated once and is used in both get_null_defines and check_header. Thus, removing in get_null_defines renders the subsequent use in check_header (which has a corresponding removal) insecure. However, since check_header is called many times, for security it wasn't in fact sufficient to remove the remove call in get_null_defines; the second and subsequent calls to check_header would still be insecure. Thus, I am testing this revised patch that (for both affected tests) adds a remove call in main and removes that in check_header. (Each use of the file is via a call to system running a command redirecting to the file, so it gets truncated each time; it just needs to remain in existence between the calls, but it doesn't matter what contents are left there after each use because the next use will replace those contents.) Avoid insecure usage of tmpnam in tests. 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-07-18 Joseph Myers <joseph@codesourcery.com> * grp/tst_fgetgrent.c: Include <unistd.h>. (main): Use mkstemp instead of tmpnam. * io/test-utime.c (main): Likewise. * posix/annexc.c (macrofile): Change to modifiable array. (main): Remove macrofile here. (get_null_defines): Use mkstemp instead of tmpnam. Do not remove macrofile here. (check_header): Do not remove macrofile here. * posix/bug-getopt1.c: Include <stdlib.h>. (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt2.c: Include <stdlib.h>. (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt3.c: Include <stdlib.h>. (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt4.c: Include <stdlib.h>. (do_test): Use mkstemp instead of tmpnam. * posix/bug-getopt5.c: Include <stdlib.h>. (do_test): Use mkstemp instead of tmpnam. * stdio-common/bug7.c: Include <stdlib.h> and <unistd.h>. (main): Use mkstemp instead of tmpnam. * stdio-common/tst-fdopen.c: Include <stdlib.h>. (main): Use mkstemp instead of tmpnam. * stdio-common/tst-ungetc.c: Include <stdlib.h>. (main): use mkstemp instead of tmpnam. * stdlib/isomac.c (macrofile): Change to modifiable array. (main): Remove macrofile here. (get_null_defines): Use mkstemp instead of tmpnam. Do not remove macrofile here. (check_header): Do not remove macrofile here. 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 <stdlib.h> #include <string.h> #include <sys/types.h> +#include <unistd.h> 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..66768db 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"; /* <aio.h>. */ static const char *const aio_syms[] = @@ -657,6 +657,8 @@ main (int argc, char *argv[]) for (h = 0; h < NUMBER_OF_HEADERS; ++h) result |= check_header (&headers[h], ignore_list); + remove (macrofile); + /* The test suite should return errors but for now this is not practical. Give a warning and ask the user to correct the bugs. */ return result; @@ -712,7 +714,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 +792,6 @@ get_null_defines (void) } result[result_len] = NULL; fclose (input); - remove (macrofile); return (const char **) result; } @@ -879,7 +886,6 @@ check_header (const struct header *header, const char **except) result |= 1; } fclose (input); - remove (macrofile); for (i = 0; i < header->nsyms; ++i) if (found[i] == 0) 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 <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <stdio.h> +#include <stdlib.h> +#include <unistd.h> 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 <stdio.h> +#include <stdlib.h> #include <unistd.h> #include <fcntl.h> @@ -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 <stdio.h> +#include <stdlib.h> #include <unistd.h> #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..0873eaa 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[] = @@ -219,6 +219,8 @@ main (int argc, char *argv[]) result |= check_header (file_name, ignore_list); } + remove (macrofile); + /* The test suite should return errors but for now this is not practical. Give a warning and ask the user to correct the bugs. */ return result; @@ -249,7 +251,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 +338,6 @@ get_null_defines (void) } result[result_len] = NULL; fclose (input); - remove (macrofile); return (const char **) result; } @@ -439,7 +446,6 @@ check_header (const char *file_name, const char **except) } } fclose (input); - remove (macrofile); return result; }
On 07/18/2018 12:47 PM, Joseph Myers wrote: > On Fri, 6 Jul 2018, Carlos O'Donell wrote: > >>> 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 <stdlib.h> >>> #include <string.h> >>> #include <sys/types.h> >>> +#include <unistd.h> >> >> Why unistd.h? > > Because of the call to close that is added. Thanks. >>> @@ -784,7 +790,6 @@ get_null_defines (void) >>> } >>> result[result_len] = NULL; >>> fclose (input); >>> - remove (macrofile); >> >> Don't we still need to remove the temporary file? > > The same temporary file has a name generated once and is used in both > get_null_defines and check_header. Thus, removing in get_null_defines > renders the subsequent use in check_header (which has a corresponding > removal) insecure. > > However, since check_header is called many times, for security it wasn't > in fact sufficient to remove the remove call in get_null_defines; the > second and subsequent calls to check_header would still be insecure. > Thus, I am testing this revised patch that (for both affected tests) adds > a remove call in main and removes that in check_header. (Each use of the > file is via a call to system running a command redirecting to the file, so > it gets truncated each time; it just needs to remain in existence between > the calls, but it doesn't matter what contents are left there after each > use because the next use will replace those contents.) Thanks, I missed that check_header() would remove it, but yes, if it's called multiple times then it should not be the site at which we call remove(). > > Avoid insecure usage of tmpnam in tests. > > 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-07-18 Joseph Myers <joseph@codesourcery.com> > > * grp/tst_fgetgrent.c: Include <unistd.h>. > (main): Use mkstemp instead of tmpnam. > * io/test-utime.c (main): Likewise. > * posix/annexc.c (macrofile): Change to modifiable array. > (main): Remove macrofile here. > (get_null_defines): Use mkstemp instead of tmpnam. Do not remove > macrofile here. > (check_header): Do not remove macrofile here. > * posix/bug-getopt1.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt2.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt3.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt4.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * posix/bug-getopt5.c: Include <stdlib.h>. > (do_test): Use mkstemp instead of tmpnam. > * stdio-common/bug7.c: Include <stdlib.h> and <unistd.h>. > (main): Use mkstemp instead of tmpnam. > * stdio-common/tst-fdopen.c: Include <stdlib.h>. > (main): Use mkstemp instead of tmpnam. > * stdio-common/tst-ungetc.c: Include <stdlib.h>. > (main): use mkstemp instead of tmpnam. > * stdlib/isomac.c (macrofile): Change to modifiable array. > (main): Remove macrofile here. > (get_null_defines): Use mkstemp instead of tmpnam. Do not remove > macrofile here. > (check_header): Do not remove macrofile here. This version looks good to me. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > 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 <stdlib.h> > #include <string.h> > #include <sys/types.h> > +#include <unistd.h> OK. > > 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); OK. > 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); OK. > diff --git a/posix/annexc.c b/posix/annexc.c > index fe3a600..66768db 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"; > > /* <aio.h>. */ > static const char *const aio_syms[] = > @@ -657,6 +657,8 @@ main (int argc, char *argv[]) > for (h = 0; h < NUMBER_OF_HEADERS; ++h) > result |= check_header (&headers[h], ignore_list); > > + remove (macrofile); OK. This makes sense to me now. > + > /* The test suite should return errors but for now this is not > practical. Give a warning and ask the user to correct the bugs. */ > return result; > @@ -712,7 +714,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 +792,6 @@ get_null_defines (void) > } > result[result_len] = NULL; > fclose (input); > - remove (macrofile); OK. > > return (const char **) result; > } > @@ -879,7 +886,6 @@ check_header (const struct header *header, const char **except) > result |= 1; > } > fclose (input); > - remove (macrofile); OK. > > for (i = 0; i < header->nsyms; ++i) > if (found[i] == 0) OK. Now annec.c looks correct to me. > 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 <unistd.h> > #include <stdio.h> > +#include <stdlib.h> > > 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 <unistd.h> > #include <stdio.h> > +#include <stdlib.h> > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> > > 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 <getopt.h> > #include <unistd.h> > #include <stdio.h> > +#include <stdlib.h> > > 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 <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > > 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 <stdio.h> > +#include <stdlib.h> > #include <unistd.h> > #include <fcntl.h> > > @@ -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 <stdio.h> > +#include <stdlib.h> > #include <unistd.h> > > #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..0873eaa 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[] = > @@ -219,6 +219,8 @@ main (int argc, char *argv[]) > result |= check_header (file_name, ignore_list); > } > > + remove (macrofile); OK. > + > /* The test suite should return errors but for now this is not > practical. Give a warning and ask the user to correct the bugs. */ > return result; > @@ -249,7 +251,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 +338,6 @@ get_null_defines (void) > } > result[result_len] = NULL; > fclose (input); > - remove (macrofile); OK. > > return (const char **) result; > } > @@ -439,7 +446,6 @@ check_header (const char *file_name, const char **except) > } > } > fclose (input); > - remove (macrofile); OK. > > return result; > } >
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 <stdlib.h> #include <string.h> #include <sys/types.h> +#include <unistd.h> 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"; /* <aio.h>. */ 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 <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <getopt.h> #include <unistd.h> #include <stdio.h> +#include <stdlib.h> 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 <stdio.h> +#include <stdlib.h> +#include <unistd.h> 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 <stdio.h> +#include <stdlib.h> #include <unistd.h> #include <fcntl.h> @@ -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 <stdio.h> +#include <stdlib.h> #include <unistd.h> #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; }