[Pkg-phototools-devel] Bug#612035: Bug#612035: vulnerability: rewrite arbitrary user file

Andreas Tille tille at debian.org
Fri Jul 8 14:08:17 UTC 2011


Hi,

I attached two debdiff files which should fullfill the requirement of a
"smallest possible patch".  Here I'm quoting the upstream author for a
description of the patch (which is included in the quilt based packaging
of 1.8 in the patch description as well):

<snip>
The original fix for this was switching from wget to libcurl, which (I
presume) is not possible in this case. Because of that, I changed feh to
create a temporary directory inside /tmp for its files.

mkdir itself will fail if the directory already exists (or is a
symlink).  Thanks to the mode of 0700, if mkdir succeeds, we can be
certain that the directory is empty and completely under our control.
wget, when callled by feh, will save its files inside that directory, so
there should be no way for an attacker to make wget save to symlinks.

There are no Backwards Compatibility problems: While I did change the
location of the temporary files, these are removed once feh exits
anyways.

If the user tells feh to keep its temporary files, they are saved in the
current working directory by default; that behaviour is not affected by
this patch.
</snip>

Feel free to upload this directly whereever it belongs to because I
will not be able to do this over the weekend.  Moreover I do not have
accordig Lenny / Squeeze chroots to build the packages in and thus
I'd prefer if somebody else could do the upload.

Kind regards

       Andreas.


On Wed, Jul 06, 2011 at 08:21:03PM +0100, Jonathan Wiltshire wrote:
> Dear maintainer,
> 
> Recently you fixed one or more security problems and as a result you closed
> this bug. These problems were not serious enough for a Debian Security
> Advisory, so they are now on my radar for fixing in the following suites
> through point releases:
> 
> lenny (5.0.9)
> squeeze (6.0.2)
> 
> Please prepare a minimal-changes upload targetting each of these suites,
> and submit a debdiff to the Release Team [0] for consideration. They will
> offer additional guidance or instruct you to upload your package.
> 
> I will happily assist you at any stage if the patch is straightforward and
> you need help or lack time. Please keep me in CC at all times so I can
> track the progress of this request.
> 
> For details of this process and the rationale, please see the original
> announcement [1] and my blog post [2].
> 
> 0: debian-release at lists.debian.org
> 1: <201101232332.11736.thijs at debian.org>
> 2: http://deb.li/prsc
> 
> Thanks,
> 
> with his security hat on:
> -- 
> Jonathan Wiltshire                                      jmw at debian.org
> Debian Developer                         http://people.debian.org/~jmw
> 
> 4096R: 0xD3524C51 / 0A55 B7C5 1223 3942 86EC  74C3 5394 479D D352 4C51



> _______________________________________________
> Pkg-phototools-devel mailing list
> Pkg-phototools-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-phototools-devel

-- 
http://fam-tille.de
-------------- next part --------------
diff -u feh-1.3.4.dfsg.1/debian/changelog feh-1.3.4.dfsg.1/debian/changelog
--- feh-1.3.4.dfsg.1/debian/changelog
+++ feh-1.3.4.dfsg.1/debian/changelog
@@ -1,3 +1,9 @@
+feh (1.3.4.dfsg.1-1lenny1) unstable; urgency=low
+
+  * Security fix (Closes: #612035)
+
+ -- Andreas Tille <tille at debian.org>  Fri, 08 Jul 2011 15:59:53 +0200
+
 feh (1.3.4.dfsg.1-1) unstable; urgency=low
 
   * acknowledge NMU (can't Britneys just get along?), thanks Paul.
only in patch2:
unchanged:
--- feh-1.3.4.dfsg.1.orig/src/main.c
+++ feh-1.3.4.dfsg.1/src/main.c
@@ -215,5 +215,9 @@
    if (opt.filelistfile)
       feh_write_filelist(filelist, opt.filelistfile);
 
+     if (opt.tmp_path && !opt.keep_http)
+		rmdir(opt.tmp_path);
+
+
    D_RETURN_(4);
 }
only in patch2:
unchanged:
--- feh-1.3.4.dfsg.1.orig/src/options.h
+++ feh-1.3.4.dfsg.1/src/options.h
@@ -89,6 +89,8 @@
    char *menu_style;
    char *caption_path;
 
+   char *tmp_path;
+
    gib_style *menu_style_l;
 
    unsigned char next_button;
only in patch2:
unchanged:
--- feh-1.3.4.dfsg.1.orig/src/options.c
+++ feh-1.3.4.dfsg.1/src/options.c
@@ -55,6 +55,7 @@
    opt.thumb_h = 60;
    opt.menu_font = estrdup(DEFAULT_MENU_FONT);
    opt.font = estrdup(DEFAULT_FONT);
+   opt.tmp_path = NULL;
    opt.menu_bg = estrdup(PREFIX "/share/feh/images/menubg_default.png");
    opt.menu_style = estrdup(PREFIX "/share/feh/fonts/menu.style");
    opt.menu_border = 4;
only in patch2:
unchanged:
--- feh-1.3.4.dfsg.1.orig/src/imlib.c
+++ feh-1.3.4.dfsg.1/src/imlib.c
@@ -263,7 +263,10 @@
    char *newurl = NULL;
    char randnum[20];
    int rnum;
-   char *path = NULL;
+   static char *path = NULL;
+   char num[10];
+   char cppid[10];
+   long int i = 1;
 
    D_ENTER(4);
 
@@ -275,7 +278,21 @@
          path = "";
    }
    else
-      path = "/tmp/";
+      snprintf(cppid, sizeof(cppid), "%06ld", (long) getpid());
+
+	while ((path == NULL) && (i < 9999)) {
+		snprintf(num, sizeof(num), "%06ld", i++);
+
+		path = estrjoin("", "/tmp/feh", "_", cppid, "_", num, "/", NULL);
+		if (mkdir(path, 0700) == -1) {
+			free(path);
+			path = NULL;
+		} else
+		opt.tmp_path = path;
+	}
+	if (path == NULL)
+		weprintf("Failed to create temporary directory:");
+
 
    basename = strrchr(url, '/') + 1;
    tmpname = feh_unique_filename(path, basename);
-------------- next part --------------
diff -Nru feh-1.8/debian/changelog feh-1.8/debian/changelog
--- feh-1.8/debian/changelog	2010-06-28 09:20:50.000000000 +0200
+++ feh-1.8/debian/changelog	2011-07-08 15:52:57.000000000 +0200
@@ -1,3 +1,9 @@
+feh (1.8-1squeeze1) unstable; urgency=low
+
+  * debian/patches/fix-612035.patch: Security fix (Closes: #612035)
+
+ -- Andreas Tille <tille at debian.org>  Fri, 08 Jul 2011 15:51:32 +0200
+
 feh (1.8-1) unstable; urgency=low
 
   * New upstream version
diff -Nru feh-1.8/debian/patches/fix-612035.patch feh-1.8/debian/patches/fix-612035.patch
--- feh-1.8/debian/patches/fix-612035.patch	1970-01-01 01:00:00.000000000 +0100
+++ feh-1.8/debian/patches/fix-612035.patch	2011-07-08 15:51:21.000000000 +0200
@@ -0,0 +1,89 @@
+Author: Daniel Friesel <derf at finalrewind.org>
+Description: About this patch
+ The original fix for this was switching from wget to libcurl, which (I
+ presume) is not possible in this case. Because of that, I changed feh to
+ create a temporary directory inside /tmp for its files.
+ .
+ mkdir itself will fail if the directory already exists (or is a
+ symlink).  Thanks to the mode of 0700, if mkdir succeeds, we can be
+ certain that the directory is empty and completely under our control.
+ wget, when callled by feh, will save its files inside that directory, so
+ there should be no way for an attacker to make wget save to symlinks.
+ .
+ There are no Backwards Compatibility problems: While I did change the
+ location of the temporary files, these are removed once feh exits
+ anyways.
+ .
+ If the user tells feh to keep its temporary files, they are saved in the
+ current working directory by default; that behaviour is not affected by
+ this patch.
+Date: Fri, 8 Jul 2011 13:57:21 +0200 
+--- feh-1.8.orig/src/imlib.c
++++ feh-1.8/src/imlib.c
+@@ -232,7 +232,10 @@
+ {
+ 	char *tmpname;
+ 	char *basename;
+-	char *path = NULL;
++	static char *path = NULL;
++	char num[10];
++	char cppid[10];
++	long int i = 1;
+ 
+ 	if (opt.keep_http) {
+ 		if (opt.output_dir)
+@@ -240,7 +243,21 @@
+ 		else
+ 			path = "";
+ 	} else
+-		path = "/tmp/";
++		snprintf(cppid, sizeof(cppid), "%06ld", (long) getpid());
++
++	while ((path == NULL) && (i < 9999)) {
++		snprintf(num, sizeof(num), "%06ld", i++);
++
++		path = estrjoin("", "/tmp/feh", "_", cppid, "_", num, "/", NULL);
++
++		if (mkdir(path, 0700) == -1) {
++			free(path);
++			path = NULL;
++		} else
++			opt.tmp_path = path;
++	}
++	if (path == NULL)
++		weprintf("Failed to create temporary directory:");
+ 
+ 	basename = strrchr(url, '/') + 1;
+ 	tmpname = feh_unique_filename(path, basename);
+--- feh-1.8.orig/src/main.c
++++ feh-1.8/src/main.c
+@@ -190,5 +190,8 @@
+ 	if (opt.filelistfile)
+ 		feh_write_filelist(filelist, opt.filelistfile);
+ 
++	if (opt.tmp_path && !opt.keep_http)
++		rmdir(opt.tmp_path);
++
+ 	return;
+ }
+--- feh-1.8.orig/src/options.c
++++ feh-1.8/src/options.c
+@@ -56,6 +56,7 @@
+ 	opt.thumb_redraw = 10;
+ 	opt.menu_font = estrdup(DEFAULT_MENU_FONT);
+ 	opt.font = NULL;
++	opt.tmp_path = NULL;
+ 	opt.image_bg = estrdup("default");
+ 	opt.menu_bg = estrdup(PREFIX "/share/feh/images/menubg_default.png");
+ 	opt.menu_style = estrdup(PREFIX "/share/feh/fonts/menu.style");
+--- feh-1.8.orig/src/options.h
++++ feh-1.8/src/options.h
+@@ -88,6 +88,8 @@
+ 	char *caption_path;
+ 	char *start_list_at;
+ 
++	char *tmp_path;
++
+ 	gib_style *menu_style_l;
+ 
+ 	unsigned char pan_button;
diff -Nru feh-1.8/debian/patches/series feh-1.8/debian/patches/series
--- feh-1.8/debian/patches/series	2010-06-28 09:22:46.000000000 +0200
+++ feh-1.8/debian/patches/series	2011-07-08 15:47:57.000000000 +0200
@@ -1 +1,2 @@
 debian-changes-1.8-1
+fix-612035.patch


More information about the Pkg-phototools-devel mailing list