- First reaction to bug #387. This version has checked MapUrl mappings.

4 files changed, 71 insertions, 8 deletions

diff --git a/README b/README
index 3bb495a..6ca7176 100644
--- a/README
+++ b/README
@@ -151,4 +151,19 @@ build. It is only neccessary if the localized strings have changed or
 if translations have been added. So if one of above is true you can
 regenerate i18n-generated.h with the make target 'generate-i18n' prior
 to creating the live plugin. In this case you need the CPAN perl
-module Locale::PO installed on your system.
\ No newline at end of file
+module Locale::PO installed on your system.
+
+
+Security consideratios
+======================
+
+Live uses the tntnet MapUrl mechanism to map different request urls
+to tntnet components. One component 'content.ecpp' delivers files
+found in the file system. When given the wrong 'path' it could
+retrieve any file from the server where live runs on.  Therefore
+content.ecpp needs to be enhanced to check the paths before returning
+files.  A second measure against missuse is to limit the mappings from
+MapUrl to only valid files. In the current version this approach has
+been taken. But due to the 'dificulty' to fully understand regular
+expressions, this might get spoiled again by 'unchecked' code
+contribution.
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 2ddd802..ec88141 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,3 +1,8 @@
+2007-09-07  Dieter Hametner  <dieter@air.mittelstation.de>
+
+	* tntconfig.cpp: Checked and adapted MapUrl regular expressions
+	                 to be more live setup secure.
+
 2007-08-19  Dieter Hametner  <dh+vdr at gekrumbel dot de>
 
 	- Adapted (but not tested) live for the new localisation scheme
diff --git a/pages/content.ecpp b/pages/content.ecpp
index 5f1780a..9fdabf3 100644
--- a/pages/content.ecpp
+++ b/pages/content.ecpp
@@ -26,6 +26,8 @@ reply.setContentType(mime);
 // FileCache::ptr_type f = LiveFileCache().get("/tmp/live/" + request.getPathInfo());
 string const path(request.getPathInfo());
 
+// dsyslog("vdrlive::content: path = %s", path.c_str());
+
 FileCache::ptr_type f;
 
 string const epgImgPath(LiveSetup().GetEpgImageDir());
@@ -33,17 +35,22 @@ if (!epgImgPath.empty() && path.find(epgImgPath) != string::npos) {
 	f = LiveFileCache().get(path);
 }
 else {
+	// dsyslog("vdrlive::content: retrieve from %s", Plugin::GetConfigDirectory().c_str());
 	f = LiveFileCache().get(Plugin::GetConfigDirectory() + "/" + path);
 }
 
 if (f.get() == 0) {
+	// dsyslog("vdrlive::content: DECLINED");
 	return DECLINED;
 }
 string ctime = tnt::HttpMessage::htdate(f->ctime());
 string browserTime = request.getHeader(tnt::httpheader::ifModifiedSince);
-if (browserTime == ctime)
+if (browserTime == ctime) {
+	// dsyslog("vdrlive::content: HTTP_NOT_MODIFIED");
 	return HTTP_NOT_MODIFIED;
+}
 
+// dsyslog("vdrlive::content: send %d bytes of data", f->size());
 reply.setHeader(tnt::httpheader::lastModified, ctime);
 reply.out().write(f->data(), f->size());
 </%cpp>
diff --git a/tntconfig.cpp b/tntconfig.cpp
index 28ad521..9adab75 100644
--- a/tntconfig.cpp
+++ b/tntconfig.cpp
@@ -34,14 +34,33 @@ void TntConfig::WriteConfig()
 		throw runtime_error( builder.str() );
 	}
 
-	// XXX modularize
+	// +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++
+	// ------------------------------------------------------------------------
+	// These MapUrl statements are very security sensitive!
+	// A wrong mapping to content@ may allow retrieval of arbitrary files
+	// from your VDR system via live.
+	// Two meassures are taken against this in our implementation:
+	// 1. The MapUrls need to be checked regulary against possible exploits
+	//    One tool to do this can be found here:
+	//      http://www.lumadis.be/regex/test_regex.php
+	//    Newly inserted MapUrls should be marked with author and confirmed
+	//    by a second party. (use source code comments for this)
+	// 2. content.ecpp will be extended to validate paths it delivers to be
+	//    a.  relative to some given roots (default plugindir)
+	//    b.  not containing ../ paths components
+	// ------------------------------------------------------------------------
+	// +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++
+
+
 	file << "MapUrl ^/$ login@" << endl;
 
 	// the following redirects vdr_request URL to the component
 	// specified by the action parameter.
+	// inserted by 'tadi' -- verified with above, but not counterchecked yet!
 	file << "MapUrl ^/vdr_request/([^.]+) $1@" << endl;
 
 	// the following selects the theme specific 'theme.css' file
+	// inserted by 'tadi' -- verified with above, but not counterchecked yet!
 	file << "MapUrl ^/themes/([^/]*)/css.*/(.+\\.css) content@ themes/$1/css/$2 text/css" << endl;
 
 	// the following rules provide a search scheme for images. The first
@@ -49,6 +68,7 @@ void TntConfig::WriteConfig()
 	// 1. /themes/<theme>/img/<imgname>.<ext>
 	// 2. /img/<imgname>.<ext>
 	// 3. <imgname>.<ext> (builtin images)
+	// inserted by 'tadi' -- verified with above, but not counterchecked yet!
 	file << "MapUrl ^/themes/([^/]*)/img.*/(.+)\\.(.+) content@ themes/$1/img/$2.$3 image/$3" << endl;
 	file << "MapUrl ^/themes/([^/]*)/img.*/(.+)\\.(.+) content@ img/$2.$3 image/$3" << endl;
 	file << "MapUrl ^/themes/([^/]*)/img.*/(.+)\\.(.+) $2@" << endl;
@@ -56,16 +76,32 @@ void TntConfig::WriteConfig()
 	// Epg images
 	string const epgImgPath(LiveSetup().GetEpgImageDir());
 	if (!epgImgPath.empty()) {
-		file << "MapUrl ^/epgimages/(.*)\\.(.+) content@ " << epgImgPath << "/$1.$2 image/$2" << endl;
+		// inserted by 'winni' -- EXPLOITABLE! (checked by tadi)
+		// file << "MapUrl ^/epgimages/(.*)\\.(.+) content@ " << epgImgPath << "/$1.$2 image/$2" << endl;
+
+		// inserted by 'tadi' -- verified with above, but not counterchecked yet!
+		file << "MapUrl ^/epgimages/([^/]*)\\.([^./]+) content@ " << epgImgPath << "/$1.$2 image/$2" << endl;
 	}
 
 	// select additional (not build in) javascript.
-	file << "MapUrl ^/js([^.]*/)(.*\\.js) content@ js$1$2 text/javascript" << endl;
+	// WARNING: no path components with '.' in the name are allowed. Only
+	// the basename may contain dots and must end with '.js'
+	// inserted by 'tadi' -- verified with above, but not counterchecked yet!
+	file << "MapUrl ^/js(/[^.]*)([^/]*\\.js) content@ js$1$2 text/javascript" << endl;
 
+	// these map to 'css/basename(uri)'
+	// inserted by 'tadi' -- verified with above, but not counterchecked yet!
 	file << "MapUrl ^/css.*/(.+) content@ css/$1 text/css" << endl;
-	file << "MapUrl ^/img.*/(.+\\.png) content@ css/$1 image/png" << endl;
-	file << "MapUrl /([^/]+/.+) content@ $1" << endl;
-	file << "MapUrl /([^.]+)(\\..+)? $1@" << endl;
+	file << "MapUrl ^/img.*/(.+)\\.([^.]+) content@ img/$1.$2 image/$2" << endl;
+
+	// insecure by default: DO NOT UNKOMMENT!!!
+	// file << "MapUrl /([^/]+/.+) content@ $1" << endl;
+
+	// takes first path components without 'extension' when it does not
+	// contain '.'
+	// modified by 'tadi' -- verified with above, but not counterchecked yet!
+	file << "MapUrl ^/([^./]+)(.*)? $1@" << endl;
+
 	file << "PropertyFile " << m_propertiesPath << endl;
 	file << "SessionTimeout 86400" << endl;
 	file << "DefaultContentType \"text/html; charset=" << LiveI18n().CharacterEncoding() << "\"" << endl;