From 5f3d9f1f80af84c71baed2fd9108aa1494ecaba5 Mon Sep 17 00:00:00 2001 From: Dieter Hametner Date: Sat, 8 Sep 2007 22:53:20 +0000 Subject: [PATCH] - Fixed bug #387. content.ecpp delivers only absolute path requests without '..' in them. --- doc/ChangeLog | 8 +++++++- pages/content.ecpp | 21 +++++++++++---------- tntconfig.cpp | 22 +++++++++++++--------- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/doc/ChangeLog b/doc/ChangeLog index ec88141e..dc00cd38 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,4 +1,10 @@ -2007-09-07 Dieter Hametner +2007-09-09 Dieter Hametner + + * 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 * tntconfig.cpp: Checked and adapted MapUrl regular expressions to be more live setup secure. diff --git a/pages/content.ecpp b/pages/content.ecpp index 9fdabf3d..27d827cc 100644 --- a/pages/content.ecpp +++ b/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"); diff --git a/tntconfig.cpp b/tntconfig.cpp index 9adab75e..41c55a20 100644 --- a/tntconfig.cpp +++ b/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,7 +65,7 @@ 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. @@ -69,8 +73,8 @@ void TntConfig::WriteConfig() // 2. /img/. // 3. . (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;