On Tue, Dec 05, 2017 at 10:54:05AM +0000, Stuart Henderson wrote:
> On 2017/12/04 17:36, Sebastien Marie wrote:
> > On Mon, Dec 04, 2017 at 11:41:10AM +0100, Sebastien Marie wrote:
> > > Hi,
> > >
> > > ISSUE: "Regression: rrdtool creates folders/files with arbitrary
> > > permissions"
> > > https://github.com/oetiker/rrdtool-1.x/issues/794
> > >
> > > FIX: "Remove all occurances of umask ... this is NOT thread safe! Fix for
> > > #794"
> > >
> > > https://github.com/oetiker/rrdtool-1.x/commit/f1edd121add94fe69ac22d991748d289b5eb76ae
> > >
> > > The following diff is the fix applied on 1.7.0.
> > >
> >
> > updated diff. It looks like my local repository wasn't up-to-date.
> >
> > Sorry about that.
>
> Thanks - there are some slight problems with the upstream commit,
I copied the diff verbatim, but I agree with the problems :)
> I propose just doing this for now. What do you think?
it is good for me. thanks !
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/rrdtool/Makefile,v
> retrieving revision 1.107
> diff -u -p -r1.107 Makefile
> --- Makefile 1 Dec 2017 10:36:51 -0000 1.107
> +++ Makefile 5 Dec 2017 10:50:58 -0000
> @@ -12,8 +12,7 @@ PKGNAME-main= ${DISTNAME}
> PKGNAME-update= rrdupdate-${VERSION}
> PKGNAME-python= py-rrd-${VERSION}
> PKGNAME-ruby= ruby${MODRUBY_BINREV}-rrd-${VERSION}
> -REVISION-main= 2
> -REVISION-update= 0
> +REVISION= 4
>
> SHARED_LIBS += rrd 5.2 # 9.0
>
> Index: patches/patch-src_rrd_create_c
> ===================================================================
> RCS file: patches/patch-src_rrd_create_c
> diff -N patches/patch-src_rrd_create_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_rrd_create_c 5 Dec 2017 10:50:58 -0000
> @@ -0,0 +1,50 @@
> +$OpenBSD$
> +
> +Based on:
> +
> +From f1edd121add94fe69ac22d991748d289b5eb76ae Mon Sep 17 00:00:00 2001
> +From: Tobias Oetiker <t...@oetiker.ch>
> +Date: Sun, 11 Jun 2017 17:19:05 +0200
> +Subject: [PATCH] Remove all occurances of umask ... this is NOT thread safe!
> + Fix for #794.
> +
> +Index: src/rrd_create.c
> +--- src/rrd_create.c.orig
> ++++ src/rrd_create.c
> +@@ -1316,7 +1316,6 @@ done:
> + int write_rrd(const char *outfilename, rrd_t *out) {
> + int rc = -1;
> + char *tmpfilename = NULL;
> +- mode_t saved_umask;
> +
> + /* write out the new file */
> + #ifdef HAVE_LIBRADOS
> +@@ -1341,10 +1340,9 @@ int write_rrd(const char *outfilename, rrd_t *out) {
> + strcpy(tmpfilename, outfilename);
> + strcat(tmpfilename, "XXXXXX");
> +
> +- /* fix CWE-377 */
> +- saved_umask = umask(S_IRUSR|S_IWUSR);
> ++ /* this is 0600 according to the manual page */
> + int tmpfd = mkstemp(tmpfilename);
> +- umask(saved_umask);
> ++
> + if (tmpfd < 0) {
> + rrd_set_error("Cannot create temporary file");
> + goto done;
> +@@ -1377,13 +1375,8 @@ int write_rrd(const char *outfilename, rrd_t *out) {
> + stat_buf.st_mode = _S_IREAD | _S_IWRITE; // have to test
> it is
> + #else
> + /* an error occurred (file not found, maybe?). Anyway:
> +- set the mode to 0666 using current umask */
> +- stat_buf.st_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP |
> S_IROTH | S_IWOTH;
> +-
> +- mode_t mask = umask(0);
> +- umask(mask);
> +-
> +- stat_buf.st_mode &= ~mask;
> ++ set the mode to 0644 */
> ++ stat_buf.st_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
> + #endif
> + }
> + if (chmod(tmpfilename, stat_buf.st_mode) != 0) {
>
--
Sebastien Marie
