frodovdr
/
vdr
mirror of https://github.com/VDR4Arch/vdr.git
1
0
Fork 0
Code Issues Releases Wiki Activity

New option '-g'; fixed security hole CAN-2005-0071 when grabbing to file

This commit is contained in:
Klaus Schmidinger 2005-12-30 15:11:16 +01:00
parent 924827fcbe
commit 3f21bf20c5
7 changed files with 95 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CONTRIBUTORS
View File

@ -1370,6 +1370,8 @@ Darren Salt <linux@youmustbejoking.demon.co.uk>
'--no-kbd'
for adding '__attribute__' to functions that use printf() like parameters
for suggesting to write grabbed images to the SVDRP connection encoded in base64
for suggesting to open the file handle in the SVDRP GRAB command in a way that
it won't follow symbolic links, and to canonicalize the file name
Sean Carlos <seanc@libero.it>
for translating OSD texts to the Italian language
@ -1571,3 +1573,7 @@ Bob Withers <bwit@pobox.com>
for publishing a Base64 encoder at http://www.ruffboy.com/download.htm
(http://www.ruffboy.com/code/Base64.zip), part of which was used when writing
the cBase64Encoder class
Javier Fernández-Sanguino Peña <jfs@computer.org>
for reporting a security hole in the way the SVDRP command GRAB writes the
image file

16
HISTORY
View File

@ -3963,7 +3963,7 @@ Video Disk Recorder Revision History
commands may now be executed at any time, and the message will be displayed
(no more "pending message").
2005-12-29: Version 1.3.38
2005-12-30: Version 1.3.38
- Fixed handling second audio and Dolby Digital PIDs for encrypted channels
(was broken in version 1.3.37).
@ -4023,3 +4023,17 @@ Video Disk Recorder Revision History
(encoded in base64) if the given file name consists of only the file
extension (".jpg", ".jpeg" or ".pnm"), or if only "-" is given as file
name (based on a suggestion from Darren Salt).
- The new command line option '-g' must be given if the SVDRP command GRAB
shall be allowed to write image files to disk. The parameter to this option
must be the full path name of an existing directory, without any "..", double
'/' or symlinks. By default, or if "-g- is given, grabbing to files is
not allowed any more because of potential security risks.
- Modified the way the SVDRP command GRAB writes the grabbed image to a file
to avoid a security hole (CAN-2005-0071, reported by Javier Fernández-Sanguino
Peña):
+ The file handle is now opened in a way that it won't follow symbolic links
(suggested by Darren Salt).
+ The given file name is now canonicalized, so that it won't contain any
".." or symlinks (suggested by Darren Salt).
+ Grabbing to files is limited to the directory given in the the command
line option '-g'. By default grabbing to files is not allowed any more.

10
device.c
View File

@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and
* how to reach the author.
*
* $Id: device.c 1.113 2005/12/29 14:51:41 kls Exp $
* $Id: device.c 1.114 2005/12/30 13:48:29 kls Exp $
*/
#include "device.h"
@ -330,12 +330,12 @@ uchar *cDevice::GrabImage(int &Size, bool Jpeg, int Quality, int SizeX, int Size
bool cDevice::GrabImageFile(const char *FileName, bool Jpeg, int Quality, int SizeX, int SizeY)
{
int result = 0;
FILE *f = fopen(FileName, "wb");
if (f) {
int fd = open(FileName, O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC, DEFFILEMODE);
if (fd >= 0) {
int ImageSize;
uchar *Image = GrabImage(ImageSize, Jpeg, Quality, SizeX, SizeY);
if (Image) {
if (fwrite(Image, ImageSize, 1, f) == 1)
if (safe_write(fd, Image, ImageSize) == ImageSize)
isyslog("grabbed image to %s", FileName);
else {
LOG_ERROR_STR(FileName);
@ -345,7 +345,7 @@ bool cDevice::GrabImageFile(const char *FileName, bool Jpeg, int Quality, int Si
}
else
result |= 1;
fclose(f);
close(fd);
}
else {
LOG_ERROR_STR(FileName);

54
svdrp.c
View File

@ -10,7 +10,7 @@
* and interact with the Video Disk Recorder - or write a full featured
* graphical interface that sits on top of an SVDRP connection.
*
* $Id: svdrp.c 1.87 2005/12/30 10:27:23 kls Exp $
* $Id: svdrp.c 1.88 2005/12/30 15:11:16 kls Exp $
*/
#include "svdrp.h"
@ -361,6 +361,8 @@ const char *GetHelpPage(const char *Cmd, const char **p)
return NULL;
}
char *cSVDRP::grabImageDir = NULL;
cSVDRP::cSVDRP(int Port)
:socket(Port)
{
@ -663,6 +665,7 @@ void cSVDRP::CmdGRAB(const char *Option)
const char *delim = " \t";
char *strtok_next;
FileName = strtok_r(p, delim, &strtok_next);
// image type:
char *Extension = strrchr(FileName, '.');
if (Extension) {
if (strcasecmp(Extension, ".jpg") == 0 || strcasecmp(Extension, ".jpeg") == 0)
@ -682,6 +685,7 @@ void cSVDRP::CmdGRAB(const char *Option)
Reply(501, "Missing filename extension in \"%s\"", FileName);
return;
}
// image quality (and obsolete type):
if ((p = strtok_r(NULL, delim, &strtok_next)) != NULL) {
if (strcasecmp(p, "JPEG") == 0 || strcasecmp(p, "PNM") == 0) {
// tolerate for backward compatibility
@ -696,6 +700,7 @@ void cSVDRP::CmdGRAB(const char *Option)
}
}
}
// image size:
if ((p = strtok_r(NULL, delim, &strtok_next)) != NULL) {
if (isnumber(p))
SizeX = atoi(p);
@ -720,19 +725,52 @@ void cSVDRP::CmdGRAB(const char *Option)
Reply(501, "Unexpected parameter \"%s\"", p);
return;
}
// canonicalize the file name:
char RealFileName[PATH_MAX];
if (FileName) {
if (grabImageDir) {
char *s;
asprintf(&s, "%s/%s", grabImageDir, FileName);
FileName = s;
char *slash = strrchr(FileName, '/'); // there definitely is one
*slash = 0;
char *r = realpath(FileName, RealFileName);
*slash = '/';
if (!r) {
LOG_ERROR_STR(FileName);
Reply(501, "Illegal file name \"%s\"", FileName);
free(s);
return;
}
strcat(RealFileName, slash);
FileName = RealFileName;
free(s);
if (strncmp(FileName, grabImageDir, strlen(grabImageDir)) != 0) {
Reply(501, "Illegal file name \"%s\"", FileName);
return;
}
}
else {
Reply(550, "Grabbing to file not allowed (use \"GRAB -\" instead)");
return;
}
}
// actual grabbing:
int ImageSize;
uchar *Image = cDevice::PrimaryDevice()->GrabImage(ImageSize, Jpeg, Quality, SizeX, SizeY);
if (Image) {
if (FileName) {
FILE *f = fopen(FileName, "wb");
if (f) {
if (fwrite(Image, ImageSize, 1, f) == 1)
int fd = open(FileName, O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC, DEFFILEMODE);
if (fd >= 0) {
if (safe_write(fd, Image, ImageSize) == ImageSize) {
isyslog("grabbed image to %s", FileName);
Reply(250, "Grabbed image %s", Option);
}
else {
LOG_ERROR_STR(FileName);
Reply(451, "Can't write to '%s'", FileName);
}
fclose(f);
close(fd);
}
else {
LOG_ERROR_STR(FileName);
@ -1530,4 +1568,10 @@ bool cSVDRP::Process(void)
return false;
}
void cSVDRP::SetGrabImageDir(const char *GrabImageDir)
{
free(grabImageDir);
grabImageDir = GrabImageDir ? strdup(GrabImageDir) : NULL;
}
//TODO more than one connection???

4
svdrp.h
View File

@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and
* how to reach the author.
*
* $Id: svdrp.h 1.26 2005/11/27 15:26:42 kls Exp $
* $Id: svdrp.h 1.27 2005/12/30 14:46:38 kls Exp $
*/
#ifndef __SVDRP_H
@ -49,6 +49,7 @@ private:
int length;
char *cmdLine;
time_t lastActivity;
static char *grabImageDir;
void Close(bool Timeout = false);
bool Send(const char *s, int length = -1);
void Reply(int Code, const char *fmt, ...) __attribute__ ((format (printf, 3, 4)));
@ -87,6 +88,7 @@ public:
~cSVDRP();
bool HasConnection(void) { return file.IsOpen(); }
bool Process(void);
static void SetGrabImageDir(const char *GrabImageDir);
};
#endif //__SVDRP_H

9
vdr.1
View File

@ -8,7 +8,7 @@
.\" License as specified in the file COPYING that comes with the
.\" vdr distribution.
.\"
.\" $Id: vdr.1 1.15 2005/10/09 12:31:03 kls Exp $
.\" $Id: vdr.1 1.16 2005/12/30 15:09:01 kls Exp $
.\"
.TH vdr 1 "19 Dec 2004" "1.3.18" "Video Disk Recorder"
.SH NAME
@ -59,6 +59,13 @@ Use \fB\-E\-\fR to disable this.
If \fIfile\fR is a directory, the file \fIepg.data\fR
will be created in that directory.
.TP
.BI \-g,\ \-\-grab= dir
Write images from the SVDRP command GRAB into the
given directory \fIdir\fR. \fIdir\fR must be the full path name of an
existing directory, without any "..", double '/'
or symlinks. By default, or if \fB\-g\-\fR is given,
grabbing images to disk is disabled.
.TP
.B \-h, \-\-help
Print a help message and exit.
.TP

11
vdr.c
View File

@ -22,7 +22,7 @@
*
* The project's page is at http://www.cadsoft.de/vdr
*
* $Id: vdr.c 1.222 2005/12/18 14:38:30 kls Exp $
* $Id: vdr.c 1.223 2005/12/30 15:07:47 kls Exp $
*/
#include <getopt.h>
@ -144,6 +144,7 @@ int main(int argc, char *argv[])
{ "daemon", no_argument, NULL, 'd' },
{ "device", required_argument, NULL, 'D' },
{ "epgfile", required_argument, NULL, 'E' },
{ "grab", required_argument, NULL, 'g' },
{ "help", no_argument, NULL, 'h' },
{ "lib", required_argument, NULL, 'L' },
{ "lirc", optional_argument, NULL, 'l' | 0x100 },
@ -164,7 +165,7 @@ int main(int argc, char *argv[])
};
int c;
while ((c = getopt_long(argc, argv, "a:c:dD:E:hl:L:mp:P:r:s:t:v:Vw:", long_options, NULL)) != -1) {
while ((c = getopt_long(argc, argv, "a:c:dD:E:g:hl:L:mp:P:r:s:t:v:Vw:", long_options, NULL)) != -1) {
switch (c) {
case 'a': AudioCommand = optarg;
break;
@ -183,6 +184,8 @@ int main(int argc, char *argv[])
break;
case 'E': EpgDataFileName = (*optarg != '-' ? optarg : NULL);
break;
case 'g': cSVDRP::SetGrabImageDir(*optarg != '-' ? optarg : NULL);
break;
case 'h': DisplayHelp = true;
break;
case 'l': {
@ -290,6 +293,10 @@ int main(int argc, char *argv[])
" '-E-' disables this\n"
" if FILE is a directory, the default EPG file will be\n"
" created in that directory\n"
" -g DIR --grab=DIR write images from the SVDRP command GRAB into the\n"
" given DIR; DIR must be the full path name of an\n"
" existing directory, without any \"..\", double '/'\n"
" or symlinks (default: none, same as -g-)\n"
" -h, --help print this help and exit\n"
" -l LEVEL, --log=LEVEL set log level (default: 3)\n"
" 0 = no logging, 1 = errors only,\n"