From 7813337cad75e71e76dbd1d4492ca0d53b523d61 Mon Sep 17 00:00:00 2001 From: Dieter Hametner Date: Thu, 6 Sep 2007 23:11:45 +0000 Subject: - First reaction to bug #387. This version has checked MapUrl mappings. --- README | 17 ++++++++++++++++- doc/ChangeLog | 5 +++++ pages/content.ecpp | 9 ++++++++- tntconfig.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++------ 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 + + * tntconfig.cpp: Checked and adapted MapUrl regular expressions + to be more live setup secure. + 2007-08-19 Dieter Hametner - 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()); 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//img/. // 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.*/(.+)\\.(.+) $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; -- cgit v1.2.3