- Fixed bug #387. content.ecpp delivers only absolute path requests

  without '..' in them.

doc/ChangeLog

@@ -1,4 +1,10 @@
-2007-09-07  Dieter Hametner  <[email protected]>
+2007-09-09  Dieter Hametner  <dh+vdr at gekrumbel dot de>
+
+	* tntconfig.cpp: allways give absolute paths to content.ecpp
+	* pages/content.ecpp: check for absolute paths which don't contain
+		upward references (e.g. '../') and deny such requests.
+
+2007-09-07  Dieter Hametner  <dh+vdr at gekrumbel dot de>
 
 	* tntconfig.cpp: Checked and adapted MapUrl regular expressions
 	                 to be more live setup secure.

pages/content.ecpp

@@ -23,21 +23,22 @@ if (request.getArgsCount() > 0) {
 reply.setContentType(mime);
 // dsyslog("vdrlive::content::mimetype(%s)", mime.c_str());
 
-// 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());
-if (!epgImgPath.empty() && path.find(epgImgPath) != string::npos) {
-	f = LiveFileCache().get(path);
+// security checking of path. In order to not allow exploits the
+// path must be absolute and not contain any upward references (e.g '../')
+if (path.empty()) {
+	return HTTP_BAD_REQUEST;
 }
-else {
-	// dsyslog("vdrlive::content: retrieve from %s", Plugin::GetConfigDirectory().c_str());
-	f = LiveFileCache().get(Plugin::GetConfigDirectory() + "/" + path);
+if ('/' != path[0]) {
+	return HTTP_BAD_REQUEST;
 }
+if (string::npos != path.find("../", 1)) {
+	return HTTP_BAD_REQUEST;
+}
+
+FileCache::ptr_type f = LiveFileCache().get(path);
 
 if (f.get() == 0) {
 	// dsyslog("vdrlive::content: DECLINED");

tntconfig.cpp

@@ -23,8 +23,10 @@ void TntConfig::WriteConfig()
 {
 	WriteProperties();
 
+	string const configDir(Plugin::GetConfigDirectory());
+
 	ostringstream builder;
-	builder << Plugin::GetConfigDirectory() << "/httpd.config";
+	builder << configDir << "/httpd.config";
 	m_configPath = builder.str();
 
 	ofstream file( m_configPath.c_str(), ios::out | ios::trunc );
@@ -45,9 +47,11 @@ void TntConfig::WriteConfig()
 	//      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)
+	// 2. content.ecpp checks the given path to be
+	//    a.  an absolute path starting at /
 	//    b.  not containing ../ paths components
+    //    In order to do so, the MapUrl statements must create absolute
+	//    path arguments to content@
 	// ------------------------------------------------------------------------
 	// +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++ CAUTION +++
 
@@ -61,16 +65,16 @@ void TntConfig::WriteConfig()
 
 	// 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;
+	file << "MapUrl ^/themes/([^/]*)/css.*/(.+\\.css) content@ " << configDir << "/themes/$1/css/$2 text/css" << endl;
 
 	// the following rules provide a search scheme for images. The first
 	// rule where a image is found, terminates the search.
 	// 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.*/(.+)\\.(.+) content@ " << configDir << "/themes/$1/img/$2.$3 image/$3" << endl;
+	file << "MapUrl ^/themes/([^/]*)/img.*/(.+)\\.(.+) content@ " << configDir << "/img/$2.$3 image/$3" << endl;
 	file << "MapUrl ^/themes/([^/]*)/img.*/(.+)\\.(.+) $2@" << endl;
 
 	// Epg images
@@ -87,12 +91,12 @@ void TntConfig::WriteConfig()
 	// 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;
+	file << "MapUrl ^/js(/[^.]*)([^/]*\\.js) content@ " << configDir << "/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.*/(.+)\\.([^.]+) content@ img/$1.$2 image/$2" << endl;
+	file << "MapUrl ^/css.*/(.+) content@ " << configDir << "/css/$1 text/css" << endl;
+	file << "MapUrl ^/img.*/(.+)\\.([^.]+) content@ " << configDir << "/img/$1.$2 image/$2" << endl;
 
 	// insecure by default: DO NOT UNKOMMENT!!!
 	// file << "MapUrl /([^/]+/.+) content@ $1" << endl;