From 095aca3eaea4cbc2237d2b3ad3d63fdad54eb2b7 Mon Sep 17 00:00:00 2001
From: Kyle Ambroff <ambroff@lindenlab.com>
Date: Tue, 7 Oct 2008 20:50:30 +0000
Subject: [PATCH] svn merge -r98039:98711
svn+ssh://svn.lindenlab.com/svn/linden/branches/Branch_1-24-Server -->
release
Merging various security fixes from Branch_1-24-Server.
Related to RequestXfer exploit:
* DEV-21706 (SEC-188): llParticleSystem can be used to obtain asset id.
* DEV-21767: Migrate RequestXfer to TCP-only
* DEV-21765: Fix RequestXfer traversal exploit
* DEV-21775: LLXferManager::processFileRequest() still has file vulnerabilities
Various fixes:
* fix for VFS memory corruption in llvfs.
* Bump server version to 1.24.9.
Landstore fixes:
* Passing locale to fulfill-order-item from region reservation fulfillment.
---
indra/llcommon/llversionserver.h | 4 +-
indra/llmessage/llxfermanager.cpp | 119 ++++++++++++++++++++++++--
indra/llmessage/llxfermanager.h | 16 ++++
indra/llvfs/llvfs.cpp | 2 +-
indra/newview/llfloaterregioninfo.cpp | 2 +
indra/newview/llviewermessage.cpp | 5 ++
scripts/messages/message_template.msg | 3 +-
7 files changed, 140 insertions(+), 11 deletions(-)
diff --git a/indra/llcommon/llversionserver.h b/indra/llcommon/llversionserver.h
index de2cbde0e19..425df83c404 100644
--- a/indra/llcommon/llversionserver.h
+++ b/indra/llcommon/llversionserver.h
@@ -34,8 +34,8 @@
const S32 LL_VERSION_MAJOR = 1;
const S32 LL_VERSION_MINOR = 24;
-const S32 LL_VERSION_PATCH = 7;
-const S32 LL_VERSION_BUILD = 97877;
+const S32 LL_VERSION_PATCH = 9;
+const S32 LL_VERSION_BUILD = 98650;
const char * const LL_CHANNEL = "Second Life Server";
diff --git a/indra/llmessage/llxfermanager.cpp b/indra/llmessage/llxfermanager.cpp
index 2afb9845c08..380ec703ced 100644
--- a/indra/llmessage/llxfermanager.cpp
+++ b/indra/llmessage/llxfermanager.cpp
@@ -713,6 +713,78 @@ void LLXferManager::sendConfirmPacket (LLMessageSystem *mesgsys, U64 id, S32 pac
///////////////////////////////////////////////////////////
+static bool find_and_remove(std::multiset<std::string>& files,
+ const std::string& filename)
+{
+ std::multiset<std::string>::iterator ptr;
+ if ( (ptr = files.find(filename)) != files.end())
+ {
+ //erase(filename) erases *all* entries with that key
+ files.erase(ptr);
+ return true;
+ }
+ return false;
+}
+
+void LLXferManager::expectFileForRequest(const std::string& filename)
+{
+ mExpectedRequests.insert(filename);
+}
+
+bool LLXferManager::validateFileForRequest(const std::string& filename)
+{
+ return find_and_remove(mExpectedRequests, filename);
+}
+
+void LLXferManager::expectFileForTransfer(const std::string& filename)
+{
+ mExpectedTransfers.insert(filename);
+}
+
+bool LLXferManager::validateFileForTransfer(const std::string& filename)
+{
+ return find_and_remove(mExpectedTransfers, filename);
+}
+
+static bool remove_prefix(std::string& filename, const std::string& prefix)
+{
+ if (std::equal(prefix.begin(), prefix.end(), filename.begin()))
+ {
+ filename = filename.substr(prefix.length());
+ return true;
+ }
+ return false;
+}
+
+static bool verify_cache_filename(const std::string& filename)
+{
+ //NOTE: This routine is only used to check file names that our own
+ // code places in the cache directory. As such, it can be limited
+ // to this very restrictive file name pattern. It does not need to
+ // handle other characters.
+
+ size_t len = filename.size();
+ //const boost::regex expr("[a-zA-Z0-9][-_.a-zA-Z0-9]<0,49>");
+ if (len < 1 || len > 50)
+ {
+ return false;
+ }
+ for(unsigned i=0; i<len; ++i)
+ {
+ char c = filename[i];
+ bool ok = isalnum(c);
+ if (!ok && i > 0)
+ {
+ ok = '_'==c || '-'==c || '.'==c;
+ }
+ if (!ok)
+ {
+ return false;
+ }
+ }
+ return true;
+}
+
void LLXferManager::processFileRequest (LLMessageSystem *mesgsys, void ** /*user_data*/)
{
@@ -734,16 +806,11 @@ void LLXferManager::processFileRequest (LLMessageSystem *mesgsys, void ** /*user
mesgsys->getStringFast(_PREHASH_XferID, _PREHASH_Filename, local_filename);
- U8 local_path_u8;
- mesgsys->getU8("XferID", "FilePath", local_path_u8);
- if( local_path_u8 < (U8)LL_PATH_LAST )
{
+ U8 local_path_u8;
+ mesgsys->getU8("XferID", "FilePath", local_path_u8);
local_path = (ELLPath)local_path_u8;
}
- else
- {
- llwarns << "Invalid file path in LLXferManager::processFileRequest() " << (U32)local_path_u8 << llendl;
- }
mesgsys->getUUIDFast(_PREHASH_XferID, _PREHASH_VFileID, uuid);
mesgsys->getS16Fast(_PREHASH_XferID, _PREHASH_VFileType, type_s16);
@@ -782,6 +849,43 @@ void LLXferManager::processFileRequest (LLMessageSystem *mesgsys, void ** /*user
}
else if (!local_filename.empty())
{
+ // See DEV-21775 for detailed security issues
+
+ if (local_path == LL_PATH_NONE)
+ {
+ // this handles legacy simulators that are passing objects
+ // by giving a filename that explicitly names the cache directory
+ static const std::string legacy_cache_prefix = "data/";
+ if (remove_prefix(local_filename, legacy_cache_prefix))
+ {
+ local_path = LL_PATH_CACHE;
+ }
+ }
+
+ switch (local_path)
+ {
+ case LL_PATH_NONE:
+ if(!validateFileForTransfer(local_filename))
+ {
+ llwarns << "SECURITY: Unapproved filename '" << local_filename << llendl;
+ return;
+ }
+ break;
+
+ case LL_PATH_CACHE:
+ if(!verify_cache_filename(local_filename))
+ {
+ llwarns << "SECURITY: Illegal cache filename '" << local_filename << llendl;
+ return;
+ }
+ break;
+
+ default:
+ llwarns << "SECURITY: Restricted file dir enum: " << (U32)local_path << llendl;
+ return;
+ }
+
+
std::string expanded_filename = gDirUtilp->getExpandedFilename( local_path, local_filename );
llinfos << "starting file transfer: " << expanded_filename << " to " << mesgsys->getSender() << llendl;
@@ -861,6 +965,7 @@ void LLXferManager::processFileRequest (LLMessageSystem *mesgsys, void ** /*user
}
}
+
///////////////////////////////////////////////////////////
void LLXferManager::processConfirmation (LLMessageSystem *mesgsys, void ** /*user_data*/)
diff --git a/indra/llmessage/llxfermanager.h b/indra/llmessage/llxfermanager.h
index 1a3e37102f2..62c5321823d 100644
--- a/indra/llmessage/llxfermanager.h
+++ b/indra/llmessage/llxfermanager.h
@@ -108,6 +108,8 @@ class LLXferManager
// implementation methods
virtual void startPendingDownloads();
virtual void addToList(LLXfer* xferp, LLXfer*& head, BOOL is_priority);
+ std::multiset<std::string> mExpectedTransfers; // files that are authorized to transfer out
+ std::multiset<std::string> mExpectedRequests; // files that are authorized to be downloaded on top of
public:
LLXferManager(LLVFS *vfs);
@@ -168,6 +170,20 @@ class LLXferManager
const LLHost& remote_host,
void (*callback)(void**,S32,LLExtStat), void** user_data,
BOOL is_priority = FALSE);
+ /**
+ When arbitrary files are requested to be transfered (by giving a dir of LL_PATH_NONE)
+ they must be "expected", but having something pre-authorize them. This pair of functions
+ maintains a pre-authorized list. The first function adds something to the list, the second
+ checks if is authorized, removing it if so. In this way, a file is only authorized for
+ a single use.
+ */
+ virtual void expectFileForTransfer(const std::string& filename);
+ virtual bool validateFileForTransfer(const std::string& filename);
+ /**
+ Same idea, but for the viewer about to call InitiateDownload to track what it requested.
+ */
+ virtual void expectFileForRequest(const std::string& filename);
+ virtual bool validateFileForRequest(const std::string& filename);
/*
// xfer request (may be memory or file)
diff --git a/indra/llvfs/llvfs.cpp b/indra/llvfs/llvfs.cpp
index ef9abd99e93..ea2ff67c0ec 100644
--- a/indra/llvfs/llvfs.cpp
+++ b/indra/llvfs/llvfs.cpp
@@ -1266,7 +1266,7 @@ void LLVFS::eraseBlockLength(LLVFSBlock *block)
}
if(!found_block)
{
- llwarns << "eraseBlock could not find block" << llendl;
+ llerrs << "eraseBlock could not find block" << llendl;
}
}
diff --git a/indra/newview/llfloaterregioninfo.cpp b/indra/newview/llfloaterregioninfo.cpp
index f6dab5391ae..e5193b83146 100644
--- a/indra/newview/llfloaterregioninfo.cpp
+++ b/indra/newview/llfloaterregioninfo.cpp
@@ -1306,6 +1306,7 @@ void LLPanelRegionTerrainInfo::onClickDownloadRaw(void* data)
return;
}
std::string filepath = picker.getFirstFile();
+ gXferManager->expectFileForRequest(filepath);
LLPanelRegionTerrainInfo* self = (LLPanelRegionTerrainInfo*)data;
strings_t strings;
@@ -1325,6 +1326,7 @@ void LLPanelRegionTerrainInfo::onClickUploadRaw(void* data)
return;
}
std::string filepath = picker.getFirstFile();
+ gXferManager->expectFileForTransfer(filepath);
LLPanelRegionTerrainInfo* self = (LLPanelRegionTerrainInfo*)data;
strings_t strings;
diff --git a/indra/newview/llviewermessage.cpp b/indra/newview/llviewermessage.cpp
index b6c524065c4..72bc991a240 100644
--- a/indra/newview/llviewermessage.cpp
+++ b/indra/newview/llviewermessage.cpp
@@ -5157,6 +5157,11 @@ void process_initiate_download(LLMessageSystem* msg, void**)
msg->getString("FileData", "SimFilename", sim_filename);
msg->getString("FileData", "ViewerFilename", viewer_filename);
+ if (!gXferManager->validateFileForRequest(viewer_filename))
+ {
+ llwarns << "SECURITY: Unauthorized download to local file " << viewer_filename << llendl;
+ return;
+ }
gXferManager->requestFile(viewer_filename,
sim_filename,
LL_PATH_NONE,
diff --git a/scripts/messages/message_template.msg b/scripts/messages/message_template.msg
index 9c92906e583..e67f540167e 100644
--- a/scripts/messages/message_template.msg
+++ b/scripts/messages/message_template.msg
@@ -8852,4 +8852,5 @@ version 2.0
{ ObjectLocalID U32 }
{ IncludeInSearch BOOL }
}
-}
\ No newline at end of file
+}
+
--
GitLab