aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWolfgang Schupp <w.schupp@a1.net>2018-10-13 16:55:17 +0200
committerGitHub <noreply@github.com>2018-10-13 16:55:17 +0200
commit4ae00ac9cecf2cb6af9bf4351167446acd2b63d7 (patch)
tree79c4fbe9d0870f2214218b3c5a4516c999a6bd9d
parent91f5e6ebfbaa51e232434f7d0f0d01356d5afaad (diff)
parent1482610efe91d5ba5eb26751b7aabb8ebb164c23 (diff)
Merge pull request #14501 from wsnipex/CVE-2017-5982
fix: Cve 2017 5982
-rw-r--r--xbmc/network/WebServer.cpp5
-rw-r--r--xbmc/network/httprequesthandler/HTTPImageHandler.cpp4
-rw-r--r--xbmc/network/httprequesthandler/HTTPWebinterfaceHandler.cpp4
-rw-r--r--xbmc/utils/FileUtils.cpp109
-rw-r--r--xbmc/utils/FileUtils.h1
5 files changed, 120 insertions, 3 deletions
diff --git a/xbmc/network/WebServer.cpp b/xbmc/network/WebServer.cpp
index f3e8d4124e..f63b336b39 100644
--- a/xbmc/network/WebServer.cpp
+++ b/xbmc/network/WebServer.cpp
@@ -28,6 +28,7 @@
#include "URL.h"
#include "Util.h"
#include "utils/Base64.h"
+#include "utils/FileUtils.h"
#include "utils/log.h"
#include "utils/Mime.h"
#include "utils/StringUtils.h"
@@ -734,6 +735,10 @@ int CWebServer::CreateFileDownloadResponse(const std::shared_ptr<IHTTPRequestHan
std::shared_ptr<XFILE::CFile> file = std::make_shared<XFILE::CFile>();
std::string filePath = handler->GetResponseFile();
+ // access check
+ if (!CFileUtils::CheckFileAccessAllowed(filePath))
+ return SendErrorResponse(request, MHD_HTTP_NOT_FOUND, request.method);
+
if (!file->Open(filePath, XFILE::READ_NO_CACHE))
{
CLog::Log(LOGERROR, "CWebServer[%hu]: Failed to open %s", m_port, filePath.c_str());
diff --git a/xbmc/network/httprequesthandler/HTTPImageHandler.cpp b/xbmc/network/httprequesthandler/HTTPImageHandler.cpp
index a0417e6037..75e89b1997 100644
--- a/xbmc/network/httprequesthandler/HTTPImageHandler.cpp
+++ b/xbmc/network/httprequesthandler/HTTPImageHandler.cpp
@@ -10,6 +10,8 @@
#include "URL.h"
#include "filesystem/ImageFile.h"
#include "network/WebServer.h"
+#include "utils/FileUtils.h"
+
CHTTPImageHandler::CHTTPImageHandler(const HTTPRequest &request)
: CHTTPFileHandler(request)
@@ -24,7 +26,7 @@ CHTTPImageHandler::CHTTPImageHandler(const HTTPRequest &request)
XFILE::CImageFile imageFile;
const CURL pathToUrl(file);
- if (imageFile.Exists(pathToUrl))
+ if (imageFile.Exists(pathToUrl) && CFileUtils::CheckFileAccessAllowed(file))
{
responseStatus = MHD_HTTP_OK;
struct __stat64 statBuffer;
diff --git a/xbmc/network/httprequesthandler/HTTPWebinterfaceHandler.cpp b/xbmc/network/httprequesthandler/HTTPWebinterfaceHandler.cpp
index 32edaefabb..6c091783ab 100644
--- a/xbmc/network/httprequesthandler/HTTPWebinterfaceHandler.cpp
+++ b/xbmc/network/httprequesthandler/HTTPWebinterfaceHandler.cpp
@@ -13,6 +13,7 @@
#include "addons/Webinterface.h"
#include "filesystem/Directory.h"
#include "filesystem/File.h"
+#include "utils/FileUtils.h"
#include "utils/StringUtils.h"
#include "utils/URIUtils.h"
@@ -61,6 +62,9 @@ int CHTTPWebinterfaceHandler::ResolveUrl(const std::string &url, std::string &pa
}
}
+ if (!CFileUtils::CheckFileAccessAllowed(path))
+ return MHD_HTTP_NOT_FOUND;
+
if (!XFILE::CFile::Exists(path))
return MHD_HTTP_NOT_FOUND;
diff --git a/xbmc/utils/FileUtils.cpp b/xbmc/utils/FileUtils.cpp
index 771864d6de..9036e912a9 100644
--- a/xbmc/utils/FileUtils.cpp
+++ b/xbmc/utils/FileUtils.cpp
@@ -5,6 +5,7 @@
* SPDX-License-Identifier: GPL-2.0-or-later
* See LICENSES/README.md for more information.
*/
+
#include "FileUtils.h"
#include "ServiceBroker.h"
#include "guilib/GUIKeyboardFactory.h"
@@ -13,9 +14,9 @@
#include "JobManager.h"
#include "FileOperationJob.h"
#include "URIUtils.h"
-#include "filesystem/StackDirectory.h"
#include "filesystem/MultiPathDirectory.h"
-#include <vector>
+#include "filesystem/SpecialProtocol.h"
+#include "filesystem/StackDirectory.h"
#include "settings/MediaSourceSettings.h"
#include "Util.h"
#include "StringUtils.h"
@@ -24,6 +25,13 @@
#include "settings/SettingsComponent.h"
#include "utils/Variant.h"
+#if defined(TARGET_WINDOWS)
+#include "platform/win32/WIN32Util.h"
+#include "utils/CharsetConverter.h"
+#endif
+
+#include <vector>
+
using namespace XFILE;
bool CFileUtils::DeleteItem(const std::string &strPath)
@@ -202,3 +210,100 @@ CDateTime CFileUtils::GetModificationDate(const std::string& strFileNameAndPath,
}
return dateAdded;
}
+
+bool CFileUtils::CheckFileAccessAllowed(const std::string &filePath)
+{
+ // DENY access to paths matching
+ const std::vector<std::string> blacklist = {
+ "passwords.xml",
+ "sources.xml",
+ "guisettings.xml",
+ "advancedsettings.xml",
+ "server.key",
+ "/.ssh/",
+ };
+ // ALLOW kodi paths
+ const std::vector<std::string> whitelist = {
+ CSpecialProtocol::TranslatePath("special://home"),
+ CSpecialProtocol::TranslatePath("special://xbmc")
+ };
+
+ // image urls come in the form of image://... sometimes with a / appended at the end
+ // strip this off to get the real file path
+ bool isImage = false;
+ std::string decodePath = CURL::Decode(filePath);
+ size_t pos = decodePath.find("image://");
+ if (pos != std::string::npos)
+ {
+ isImage = true;
+ decodePath.erase(pos, 8);
+ URIUtils::RemoveSlashAtEnd(decodePath);
+ }
+
+ // check blacklist
+ for (const auto &b : blacklist)
+ {
+ if (decodePath.find(b) != std::string::npos)
+ {
+ CLog::Log(LOGERROR,"%s denied access to %s", __FUNCTION__, decodePath.c_str());
+ return false;
+ }
+ }
+
+#if defined(TARGET_POSIX)
+ std::string whiteEntry;
+ char *fullpath = realpath(decodePath.c_str(), nullptr);
+
+ // if this is a locally existing file, check access permissions
+ if (fullpath)
+ {
+ const std::string realPath = fullpath;
+ free(fullpath);
+
+ // check whitelist
+ for (const auto &w : whitelist)
+ {
+ char *realtemp = realpath(w.c_str(), nullptr);
+ if (realtemp)
+ {
+ whiteEntry = realtemp;
+ free(realtemp);
+ }
+ if (StringUtils::StartsWith(realPath, whiteEntry))
+ return true;
+ }
+ // check sources with realPath
+ return CFileUtils::RemoteAccessAllowed(realPath);
+ }
+#elif defined(TARGET_WINDOWS)
+ CURL url(decodePath);
+ if (url.GetProtocol().empty())
+ {
+ std::wstring decodePathW;
+ g_charsetConverter.utf8ToW(decodePath, decodePathW, false);
+ CWIN32Util::AddExtraLongPathPrefix(decodePathW);
+ DWORD bufSize = GetFullPathNameW(decodePathW.c_str(), 0, nullptr, nullptr);
+ if (bufSize > 0)
+ {
+ std::wstring fullpathW;
+ fullpathW.resize(bufSize);
+ if (GetFullPathNameW(decodePathW.c_str(), bufSize, const_cast<wchar_t*>(fullpathW.c_str()), nullptr) <= bufSize - 1)
+ {
+ CWIN32Util::RemoveExtraLongPathPrefix(fullpathW);
+ std::string fullpath;
+ g_charsetConverter.wToUTF8(fullpathW, fullpath, false);
+ for (const std::string& whiteEntry : whitelist)
+ {
+ if (StringUtils::StartsWith(fullpath, whiteEntry))
+ return true;
+ }
+ return CFileUtils::RemoteAccessAllowed(fullpath);
+ }
+ }
+ }
+#endif
+ // if it isn't a local file, it must be a vfs entry
+ if (! isImage)
+ return CFileUtils::RemoteAccessAllowed(decodePath);
+ return true;
+}
diff --git a/xbmc/utils/FileUtils.h b/xbmc/utils/FileUtils.h
index 13bb162065..fe76e6aef7 100644
--- a/xbmc/utils/FileUtils.h
+++ b/xbmc/utils/FileUtils.h
@@ -14,6 +14,7 @@
class CFileUtils
{
public:
+ static bool CheckFileAccessAllowed(const std::string &filePath);
static bool DeleteItem(const CFileItemPtr &item);
static bool DeleteItem(const std::string &strPath);
static bool RenameFile(const std::string &strFile);