[vdr] [PATCH] [RFC] Stop SVDRP 'grab' command from writing outside certain directories

[Darren Salt <linux@youmustbejoking.demon.co.uk>]

This message is in MIME format which your mailer apparently does not support.
You either require a newer version of your software which supports MIME, or
a separate MIME decoding utility.  Alternatively, ask the sender of this
message to resend it in a different format.

--1295037716--1941306070--658331908
Content-Type: text/plain; charset=us-ascii

The attached patch is intended to replace the existing security fix for VDR's
SVDRP GRAB command. It allows writing below $VideoDirectory/snaps.dir and
/tmp; it rejects attempts to write elsewhere (and 

If you specify a relative pathname, your snapshot is put in snaps.dir.

If you use vdr-xine 0.7.0, you'll need to patch it too (patch attached) in
order to prevent symlink attacks. 0.6.5 and earlier require a different, more
intrusive patch, though my packaged 0.6.5 requires only
  $ sed -i -e s/O_EXCL/O_NOFOLLOW/ debian/patches/02_use_O_EXCL.dpatch
followed by a rebuild.

(Debian-format packages with these patches are being prepared.)

-- 
| Darren Salt | nr. Ashington, | d youmustbejoking,demon,co,uk
| Debian,     | Northumberland | s zap,tartarus,org
| RISC OS     | Toon Army      | @                      Say NO to UK ID cards
|                                                       http://www.no2id.net/

File is read only, 0:1

--1295037716--1941306070--658331908
Content-Type: text/plain; charset=iso-8859-1; name="svdrp-grab.patch"
Content-Disposition: attachment; filename="svdrp-grab.patch"
Content-Transfer-Encoding: quoted-printable

diff -urNad vdr-1.3.19~/dvbdevice.c vdr-1.3.19/dvbdevice.c
--- vdr-1.3.19~/dvbdevice.c
+++ vdr-1.3.19/dvbdevice.c
@@ -547,8 +547,9 @@
               Quality = 100;
 
            isyslog("grabbing to %s (%s %d %d %d)", FileName, Jpeg ? "JPEG" : "PNM", Quality, vm.width, vm.height);
-           FILE *f = fopen(FileName, "wb");
-           if (f) {
+           int fd = open (FileName, O_CREAT | O_NOFOLLOW | O_TRUNC | O_RDWR, 0644);
+           FILE *f;
+           if (fd != -1 && (f = fdopen(f, "wb"))) {
               if (Jpeg) {
                  // write JPEG file:
                  struct jpeg_compress_struct cinfo;
@@ -585,6 +586,8 @@
               }
            else {
               LOG_ERROR_STR(FileName);
+              if (fd != -1 && close (fd))
+                 LOG_ERROR_STR(FileName);
               result |= 1;
               }
            munmap(mem, msize);
diff -urNad vdr-1.3.19~/svdrp.c vdr-1.3.19/svdrp.c
--- vdr-1.3.19~/svdrp.c
+++ vdr-1.3.19/svdrp.c
@@ -599,10 +599,55 @@
         Reply(501, "Unexpected parameter \"%s\"", p);
         return;
         }
-     if (cDevice::PrimaryDevice()->GrabImage(FileName, Jpeg, Quality, SizeX, SizeY))
+
+     char *dir, *fpath = NULL;
+     asprintf (&dir, "%s/snaps.dir", VideoDirectory);
+     if (mkdir (dir, 0755) && errno != EEXIST)
+      {
+	LOG_ERROR_STR(dir);
+	Reply(451, "Grab image failed");
+	free (dir);
+	return;
+      }
+     if (*FileName != '/')
+	asprintf (&fpath, "%s/%s", dir, FileName);
+     // fpath = full pathname (not canonicalised) or NULL
+
+     char *tmp = strrchr (fpath ? fpath : FileName, '/'); // there is one
+     *tmp = 0;
+     char path[PATH_MAX];
+     if (!realpath (fpath ? fpath : FileName, path)) // canonicalise pathname
+      {
+	Reply(501, "Invalid filename");
+	free (fpath);
+	free (dir);
+	return;
+      }
+     //
+     asprintf (&tmp, "%s/%s", path, tmp + 1);
+     free (fpath);
+     fpath = tmp; // full pathname (canonicalised)
+
+     realpath (dir, path); // dir name (canonicalised)
+     if (!strncmp (fpath, path, strlen (path)) && fpath[strlen (path)] == '/')
+      {
+        /* nothing */
+      }
+     else if (strncmp (fpath, "/tmp/", 5))
+      {
+	Reply(501, "Invalid filename");
+	free (fpath);
+	free (dir);
+	return;
+      }
+     free (dir);
+
+     if (cDevice::PrimaryDevice()->GrabImage(fpath, Jpeg, Quality, SizeX, SizeY))
         Reply(250, "Grabbed image %s", Option);
      else
         Reply(451, "Grab image failed");
+
+     free (fpath);
      }
   else
      Reply(501, "Missing filename");

--1295037716--1941306070--658331908
Content-Type: text/plain; charset=iso-8859-1; name="vdr-xine-0.7.0_symlink.patch"
Content-Disposition: attachment; filename="vdr-xine-0.7.0_symlink.patch"
Content-Transfer-Encoding: quoted-printable

diff -urNad vdr-plugin-xine-0.7.0~/xineLib.c vdr-plugin-xine-0.7.0/xineLib.c
--- vdr-plugin-xine-0.7.0~/xineLib.c
+++ vdr-plugin-xine-0.7.0/xineLib.c
@@ -3254,7 +3254,7 @@
       
       bool success = false;
         
-      int outfd = ::open(FileName, O_CREAT /* | O_EXCL */ | O_TRUNC | O_RDWR, 0644);
+      int outfd = ::open(FileName, O_CREAT | O_NOFOLLOW | O_TRUNC | O_RDWR, 0644);
       if (-1 != outfd)
       {
         int sizeX = SizeX;

--1295037716--1941306070--658331908--