This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fix hardcoded /tmp paths in testing (bug 13888)
On 22/06/2018 13:17, Joseph Myers wrote:
> As noted in bug 13888, and as I noted previously in
> <https://sourceware.org/ml/libc-alpha/2000-10/msg00111.html>, various
> tests used hardcoded paths in /tmp, so posing issues for simultaneous
> test runs from different build directories.
>
> This patch fixes such uses of hardcoded file names to put them in the
> build directory instead (in the case of stdio-common/bug5 the file
> names are changed as well, to avoid a conflict with the name bug5.out
> also used for the automatic test output redirection). It also fixes
> test-installation.pl likewise (that was using filenames with $$ in
> them rather than strictly hardcoded names, but that's still not good
> practice for temporary file naming).
>
> Note that my list of files changed is not identical to that in bug
> 13888. I added tst-spawn3.c and test-installation.pl, and removed
> some tests that seem to me (now) to create temporary files securely
> (simply using /tmp is not itself a problem if the temporary files are
> handled properly with mkstemp; I haven't checked whether those tests
> used to do things insecurely). conformtest is not changed because the
> makefiles always pass a --tmpdir option so the /tmp default is
> irrelevant, and for the same reason there is no actual problem with
> nptl/tst-umask1.c because again the makefiles always override the
> default.
>
> nptl/sockperf.c is ignored because there is no code to run it;
> probably that file should actually be removed.
>
> Some tests use the mktemp function, but I think they all use it in a
> way that *is* secure (for generating names for directories / sockets /
> fifos / symlinks, where the operation using the name will not follow
> symlinks and so there is no potential for a symlink attack on the
> account running the testsuite).
>
> Some tests use the tmpnam function to generate temporary file names.
> This is in principle insecure, but not addressed by this patch (I
> consider it a separate issue from the fully hardcoded paths).
>
> Tested for x86_64.
>
> 2018-06-22 Joseph Myers <joseph@codesourcery.com>
>
> [BZ #13888]
> * posix/Makefile (CFLAGS-tst-spawn3.c): New variable.
> * posix/tst-spawn3.c (do_test): Put tst-spwan3.pid in OBJPFX, not
> /tmp.
> * scripts/test-installation.pl: Put temporary files in build
> directory, not /tmp.
> * stdio-common/Makefile (CFLAGS-bug3.c): New variable.
> (CFLAGS-bug4.c): Likewise.
> (CFLAGS-bug5.c): Likewise.
> (CFLAGS-test-fseek.c): Likewise.
> (CFLAGS-test-popen.c): Likewise.
> (CFLAGS-test_rdwr.c): Likewise.
> * stdio-common/bug3.c (main): Put temporary file in OBJPFX, not
> /tmp.
> * stdio-common/bug4.c (main): Likewise.
> * stdio-common/bug5.c (main): Likewise.
> * stdio-common/test-fseek.c (TESTFILE): Likewise.
> * stdio-common/test-popen.c (do_test): Likewise.
> * stdio-common/test_rdwr.c (main): Likewise.
LGTM, thanks.
>
> diff --git a/posix/Makefile b/posix/Makefile
> index 4a9a1b5..989e42e 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -256,6 +256,7 @@ tst-pcre-ARGS = PCRE.tests
> tst-boost-ARGS = BOOST.tests
> bug-glob1-ARGS = "$(objpfx)"
> tst-execvp3-ARGS = --test-dir=$(objpfx)
> +CFLAGS-tst-spawn3.c += -DOBJPFX=\"$(objpfx)\"
>
> testcases.h: TESTS TESTS2C.sed
> LC_ALL=C sed -f TESTS2C.sed < $< > $@T
> diff --git a/posix/tst-spawn3.c b/posix/tst-spawn3.c
> index dc6bed5..b60a783 100644
> --- a/posix/tst-spawn3.c
> +++ b/posix/tst-spawn3.c
> @@ -82,8 +82,8 @@ do_test (void)
> if (posix_spawn_file_actions_init (&a) != 0)
> FAIL_EXIT1 ("posix_spawn_file_actions_init");
>
> - /* Executes a /bin/sh echo $$ 2>&1 > /tmp/tst-spawn3.pid . */
> - const char pidfile[] = "/tmp/tst-spawn3.pid";
> + /* Executes a /bin/sh echo $$ 2>&1 > ${objpfx}tst-spawn3.pid . */
> + const char pidfile[] = OBJPFX "tst-spawn3.pid";
> if (posix_spawn_file_actions_addopen (&a, STDOUT_FILENO, pidfile, O_WRONLY |
> O_CREAT | O_TRUNC, 0644) != 0)
> FAIL_EXIT1 ("posix_spawn_file_actions_addopen");
> diff --git a/scripts/test-installation.pl b/scripts/test-installation.pl
> index 067da47..b2e4ba7 100755
> --- a/scripts/test-installation.pl
> +++ b/scripts/test-installation.pl
> @@ -79,10 +79,12 @@ arglist: while (@ARGV) {
>
> # We expect none or one argument.
> if ($#ARGV == -1) {
> + $dir = ".";
> $soversions="soversions.mk";
> $config="config.make";
> } elsif ($#ARGV == 0) {
> if (-d $ARGV[0]) {
> + $dir = $ARGV[0];
> $soversions = "$ARGV[0]/soversions.mk";
> $config = "$ARGV[0]/config.make";
> } else {
> @@ -141,8 +143,8 @@ close SOVERSIONS;
> # Create test program and link it against all
> # shared libraries
>
> -open PRG, ">/tmp/test-prg$$.c"
> - or die ("Couldn't write test file /tmp/test-prg$$.c");
> +open PRG, ">$dir/test-prg$$.c"
> + or die ("Couldn't write test file $dir/test-prg$$.c");
>
> print PRG '
> #include <stdio.h>
> @@ -154,7 +156,7 @@ int main(void) {
> ';
> close PRG;
>
> -open GCC, "$CC /tmp/test-prg$$.c $link_libs -o /tmp/test-prg$$ 2>&1 |"
> +open GCC, "$CC $dir/test-prg$$.c $link_libs -o $dir/test-prg$$ 2>&1 |"
> or die ("Couldn't execute $CC!");
>
> while (<GCC>) {
> @@ -172,7 +174,7 @@ if ($?) {
> $ok = 1;
> %found = ();
>
> -open LDD, "ldd /tmp/test-prg$$ |"
> +open LDD, "ldd $dir/test-prg$$ |"
> or die ("Couldn't execute ldd");
> while (<LDD>) {
> if (/^\s*lib/) {
> @@ -212,8 +214,8 @@ foreach (keys %versions) {
> &installation_problem unless $ok;
>
> # Finally execute the test program
> -system ("/tmp/test-prg$$") == 0
> +system ("$dir/test-prg$$") == 0
> or die ("Execution of test program failed");
>
> # Clean up after ourselves
> -unlink ("/tmp/test-prg$$", "/tmp/test-prg$$.c");
> +unlink ("$dir/test-prg$$", "$dir/test-prg$$.c");
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 4503837..738a3ce 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -145,6 +145,13 @@ CFLAGS-scanf15.c += -I../libio -I../stdlib -I../wcsmbs -I../time -I../string \
> CFLAGS-scanf17.c += -I../libio -I../stdlib -I../wcsmbs -I../time -I../string \
> -I../wctype
>
> +CFLAGS-bug3.c += -DOBJPFX=\"$(objpfx)\"
> +CFLAGS-bug4.c += -DOBJPFX=\"$(objpfx)\"
> +CFLAGS-bug5.c += -DOBJPFX=\"$(objpfx)\"
> +CFLAGS-test-fseek.c += -DOBJPFX=\"$(objpfx)\"
> +CFLAGS-test-popen.c += -DOBJPFX=\"$(objpfx)\"
> +CFLAGS-test_rdwr.c += -DOBJPFX=\"$(objpfx)\"
> +
> # tst-gets.c tests a deprecated function.
> CFLAGS-tst-gets.c += -Wno-deprecated-declarations
>
> diff --git a/stdio-common/bug3.c b/stdio-common/bug3.c
> index 6b2ed6b..62a6cab 100644
> --- a/stdio-common/bug3.c
> +++ b/stdio-common/bug3.c
> @@ -6,7 +6,7 @@ main (void)
> {
> FILE *f;
> int i;
> - const char filename[] = "/tmp/bug3.test";
> + const char filename[] = OBJPFX "bug3.test";
>
> f = fopen(filename, "w+");
> for (i=0; i<9000; i++)
> diff --git a/stdio-common/bug4.c b/stdio-common/bug4.c
> index a037786..cf7fe11 100644
> --- a/stdio-common/bug4.c
> +++ b/stdio-common/bug4.c
> @@ -10,7 +10,7 @@ main (int argc, char *argv[])
> FILE *f;
> int i;
> char buffer[31];
> - const char filename[] = "/tmp/bug4.test";
> + const char filename[] = OBJPFX "bug4.test";
>
> while ((i = getopt (argc, argv, "rw")) != -1)
> switch (i)
> diff --git a/stdio-common/bug5.c b/stdio-common/bug5.c
> index f655845..7bfe9b2 100644
> --- a/stdio-common/bug5.c
> +++ b/stdio-common/bug5.c
> @@ -14,8 +14,8 @@ main (void)
> {
> FILE *in;
> FILE *out;
> - static char inname[] = "/tmp/bug5.in";
> - static char outname[] = "/tmp/bug5.out";
> + static char inname[] = OBJPFX "bug5test.in";
> + static char outname[] = OBJPFX "bug5test.out";
> char *printbuf;
> size_t i;
> int result;
> diff --git a/stdio-common/test-fseek.c b/stdio-common/test-fseek.c
> index c313911..17cd90b 100644
> --- a/stdio-common/test-fseek.c
> +++ b/stdio-common/test-fseek.c
> @@ -17,7 +17,7 @@
>
> #include <stdio.h>
>
> -#define TESTFILE "/tmp/test.dat"
> +#define TESTFILE OBJPFX "test.dat"
>
> static int
> do_test (void)
> diff --git a/stdio-common/test-popen.c b/stdio-common/test-popen.c
> index 6e98d3c..206f212 100644
> --- a/stdio-common/test-popen.c
> +++ b/stdio-common/test-popen.c
> @@ -59,7 +59,7 @@ do_test (void)
> the perhaps incompatible new shared libraries. */
> unsetenv ("LD_LIBRARY_PATH");
>
> - output = popen ("/bin/cat >/tmp/tstpopen.tmp", "w");
> + output = popen ("/bin/cat >" OBJPFX "tstpopen.tmp", "w");
> if (output == NULL)
> {
> perror ("popen");
> @@ -69,10 +69,10 @@ do_test (void)
> write_data (output);
> wstatus = pclose (output);
> printf ("writing pclose returned %d\n", wstatus);
> - input = popen ("/bin/cat /tmp/tstpopen.tmp", "r");
> + input = popen ("/bin/cat " OBJPFX "tstpopen.tmp", "r");
> if (input == NULL)
> {
> - perror ("/tmp/tstpopen.tmp");
> + perror (OBJPFX "tstpopen.tmp");
> puts ("Test FAILED!");
> exit (1);
> }
> @@ -80,7 +80,7 @@ do_test (void)
> rstatus = pclose (input);
> printf ("reading pclose returned %d\n", rstatus);
>
> - remove ("/tmp/tstpopen.tmp");
> + remove (OBJPFX "tstpopen.tmp");
>
> errno = 0;
> output = popen ("/bin/cat", "m");
> diff --git a/stdio-common/test_rdwr.c b/stdio-common/test_rdwr.c
> index f471570..3489873 100644
> --- a/stdio-common/test_rdwr.c
> +++ b/stdio-common/test_rdwr.c
> @@ -38,7 +38,7 @@ main (int argc, char **argv)
> else
> name = *argv;
>
> - (void) sprintf (filename, "/tmp/%s.test", name);
> + (void) sprintf (filename, OBJPFX "%s.test", name);
>
> f = fopen (filename, "w+");
> if (f == NULL)
>