On 04/27/2011 07:52 AM, Reuben Thomas wrote: > On 27 April 2011 14:00, Eric Blake <ebl...@redhat.com> wrote: >> On 04/27/2011 04:05 AM, Reuben Thomas wrote: >>> On 27 April 2011 03:24, Bruno Haible <br...@clisp.org> wrote: >>>>> Does setting a 0600 umask (as glibc does) sound like a good thing to >>>>> add to the mkstemp-safer functions? >> >> Setting umask() is bad for other reasons - it is global state, > > I was being sloppy with my terminology, I meant of course file permissions. > >> The >> hardest part now is writing the m4 test to detect platforms whose >> mkstemp is insecure. > > Attached, a patch. I don't have a non-GNU system on which to test it; > I can only confirm that it works on GNU/Linux (which doesn't prove > anything really).
> commit 76d83ad4cac604b71489cf2d566db6dea65f92f3
> Author: Reuben Thomas <r...@sc3d.org>
> Date: Wed Apr 27 14:51:22 2011 +0100
>
> Use gnulib's mkstemp if the system implementation is insecure.
>
> * m4/mkstemp.m4: Add test for non-owner read/write mode bits set
> in file created by mkstemp.
>
> diff --git a/m4/mkstemp.m4 b/m4/mkstemp.m4
> index c5cd282..e071bb4 100644
> --- a/m4/mkstemp.m4
> +++ b/m4/mkstemp.m4
> @@ -10,6 +10,8 @@
> # Other systems lack mkstemp altogether.
Let's bump the serial number.
> # On OSF1/Tru64 V4.0F, the system-provided mkstemp function can create
> # only 32 files per process.
> +# On some hosts, mkstemp creates files with mode 0666, which is a security
> +# problem.
Also, it was valid in POSIX 2001, but violates POSIX 2008.
> # On systems like the above, arrange to use the replacement function.
> AC_DEFUN([gl_FUNC_MKSTEMP],
> [
> @@ -44,6 +46,25 @@ AC_DEFUN([gl_FUNC_MKSTEMP],
> close (fd);
> }
> }
> + if (result == 0)
> + {
> + char templ[] = "conftest.mkstemp/coXXXXXX";
> + int (*mkstemp_function) (char *) = mkstemp;
> + int fd = mkstemp_function (templ);
> +
> + if (fd < 0)
> + result |= 4;
> + else
> + {
> + struct stat st;
> +
> + if (stat (templ, &st) < 0)
fstat() is better.
> + result |= 8;
> + else if (st.st_mode & 0077)
False negative if umask is already restrictive. We need to first change
umask() before probing mkstemp().
> + result |= 16;
> + close (fd);
> + }
> + }
> return result;]])],
> [gl_cv_func_working_mkstemp=yes],
> [gl_cv_func_working_mkstemp=no],
We also need to document this fix. I'm working on a v2 of this patch...
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
