You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
316 lines
9.7 KiB
316 lines
9.7 KiB
Author: Ben Hutchings <ben@decadent.org.uk>
|
|
Date: Tue, 19 May 2015 02:38:40 +0100
|
|
Description: Delay creation of symlinks to prevent arbitrary file writes (CVE-2015-1038)
|
|
Bug: http://sourceforge.net/p/p7zip/bugs/147/
|
|
Bug-Debian: https://bugs.debian.org/774660
|
|
|
|
Alexander Cherepanov discovered that 7zip is susceptible to a
|
|
directory traversal vulnerability. While extracting an archive, it
|
|
will extract symlinks and then follow them if they are referenced in
|
|
further entries. This can be exploited by a rogue archive to write
|
|
files outside the current directory.
|
|
|
|
We have to create placeholder files (which we already do) and delay
|
|
creating symlinks until the end of extraction.
|
|
|
|
Due to the possibility of anti-items (deletions) in the archive, it is
|
|
possible for placeholders to be deleted and replaced before we create
|
|
the symlinks. It's not clear that this can be used for mischief, but
|
|
GNU tar guards against similar problems by checking that the placeholder
|
|
still exists and is the same inode. XXX It also checks 'birth time' but
|
|
this isn't portable. We can probably get away with comparing ctime
|
|
since we don't support hard links.
|
|
|
|
--- a/CPP/7zip/UI/Agent/Agent.cpp
|
|
+++ b/CPP/7zip/UI/Agent/Agent.cpp
|
|
@@ -424,6 +424,8 @@ STDMETHODIMP CAgentFolder::Extract(const
|
|
CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec;
|
|
UStringVector pathParts;
|
|
CProxyFolder *currentProxyFolder = _proxyFolderItem;
|
|
+ HRESULT res;
|
|
+
|
|
while (currentProxyFolder->Parent)
|
|
{
|
|
pathParts.Insert(0, currentProxyFolder->Name);
|
|
@@ -445,8 +447,11 @@ STDMETHODIMP CAgentFolder::Extract(const
|
|
(UInt64)(Int64)-1);
|
|
CUIntVector realIndices;
|
|
GetRealIndices(indices, numItems, realIndices);
|
|
- return _agentSpec->GetArchive()->Extract(&realIndices.Front(),
|
|
+ res = _agentSpec->GetArchive()->Extract(&realIndices.Front(),
|
|
realIndices.Size(), testMode, extractCallback);
|
|
+ if (res == S_OK && !extractCallbackSpec->CreateSymLinks())
|
|
+ res = E_FAIL;
|
|
+ return res;
|
|
COM_TRY_END
|
|
}
|
|
|
|
--- a/CPP/7zip/UI/Agent/ArchiveFolder.cpp
|
|
+++ b/CPP/7zip/UI/Agent/ArchiveFolder.cpp
|
|
@@ -20,6 +20,8 @@ STDMETHODIMP CAgentFolder::CopyTo(const
|
|
CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec;
|
|
UStringVector pathParts;
|
|
CProxyFolder *currentProxyFolder = _proxyFolderItem;
|
|
+ HRESULT res;
|
|
+
|
|
while (currentProxyFolder->Parent)
|
|
{
|
|
pathParts.Insert(0, currentProxyFolder->Name);
|
|
@@ -46,8 +48,11 @@ STDMETHODIMP CAgentFolder::CopyTo(const
|
|
(UInt64)(Int64)-1);
|
|
CUIntVector realIndices;
|
|
GetRealIndices(indices, numItems, realIndices);
|
|
- return _agentSpec->GetArchive()->Extract(&realIndices.Front(),
|
|
+ res = _agentSpec->GetArchive()->Extract(&realIndices.Front(),
|
|
realIndices.Size(), BoolToInt(false), extractCallback);
|
|
+ if (res == S_OK && !extractCallbackSpec->CreateSymLinks())
|
|
+ res = E_FAIL;
|
|
+ return res;
|
|
COM_TRY_END
|
|
}
|
|
|
|
--- a/CPP/7zip/UI/Client7z/Client7z.cpp
|
|
+++ b/CPP/7zip/UI/Client7z/Client7z.cpp
|
|
@@ -197,8 +197,11 @@ private:
|
|
COutFileStream *_outFileStreamSpec;
|
|
CMyComPtr<ISequentialOutStream> _outFileStream;
|
|
|
|
+ CObjectVector<NWindows::NFile::NDirectory::CDelayedSymLink> _delayedSymLinks;
|
|
+
|
|
public:
|
|
void Init(IInArchive *archiveHandler, const UString &directoryPath);
|
|
+ bool CreateSymLinks();
|
|
|
|
UInt64 NumErrors;
|
|
bool PasswordIsDefined;
|
|
@@ -392,11 +395,22 @@ STDMETHODIMP CArchiveExtractCallback::Se
|
|
}
|
|
_outFileStream.Release();
|
|
if (_extractMode && _processedFileInfo.AttribDefined)
|
|
- NFile::NDirectory::MySetFileAttributes(_diskFilePath, _processedFileInfo.Attrib);
|
|
+ NFile::NDirectory::MySetFileAttributes(_diskFilePath, _processedFileInfo.Attrib, &_delayedSymLinks);
|
|
PrintNewLine();
|
|
return S_OK;
|
|
}
|
|
|
|
+bool CArchiveExtractCallback::CreateSymLinks()
|
|
+{
|
|
+ bool success = true;
|
|
+
|
|
+ for (int i = 0; i != _delayedSymLinks.Size(); ++i)
|
|
+ success &= _delayedSymLinks[i].Create();
|
|
+
|
|
+ _delayedSymLinks.Clear();
|
|
+
|
|
+ return success;
|
|
+}
|
|
|
|
STDMETHODIMP CArchiveExtractCallback::CryptoGetTextPassword(BSTR *password)
|
|
{
|
|
--- a/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp
|
|
+++ b/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp
|
|
@@ -453,12 +453,24 @@ STDMETHODIMP CArchiveExtractCallback::Se
|
|
NumFiles++;
|
|
|
|
if (_extractMode && _fi.AttribDefined)
|
|
- NFile::NDirectory::MySetFileAttributes(_diskFilePath, _fi.Attrib);
|
|
+ NFile::NDirectory::MySetFileAttributes(_diskFilePath, _fi.Attrib, &_delayedSymLinks);
|
|
RINOK(_extractCallback2->SetOperationResult(operationResult, _encrypted));
|
|
return S_OK;
|
|
COM_TRY_END
|
|
}
|
|
|
|
+bool CArchiveExtractCallback::CreateSymLinks()
|
|
+{
|
|
+ bool success = true;
|
|
+
|
|
+ for (int i = 0; i != _delayedSymLinks.Size(); ++i)
|
|
+ success &= _delayedSymLinks[i].Create();
|
|
+
|
|
+ _delayedSymLinks.Clear();
|
|
+
|
|
+ return success;
|
|
+}
|
|
+
|
|
/*
|
|
STDMETHODIMP CArchiveExtractCallback::GetInStream(
|
|
const wchar_t *name, ISequentialInStream **inStream)
|
|
--- a/CPP/7zip/UI/Common/ArchiveExtractCallback.h
|
|
+++ b/CPP/7zip/UI/Common/ArchiveExtractCallback.h
|
|
@@ -6,6 +6,8 @@
|
|
#include "Common/MyCom.h"
|
|
#include "Common/Wildcard.h"
|
|
|
|
+#include "Windows/FileDir.h"
|
|
+
|
|
#include "../../IPassword.h"
|
|
|
|
#include "../../Common/FileStreams.h"
|
|
@@ -83,6 +85,8 @@ class CArchiveExtractCallback:
|
|
UInt64 _packTotal;
|
|
UInt64 _unpTotal;
|
|
|
|
+ CObjectVector<NWindows::NFile::NDirectory::CDelayedSymLink> _delayedSymLinks;
|
|
+
|
|
void CreateComplexDirectory(const UStringVector &dirPathParts, UString &fullPath);
|
|
HRESULT GetTime(int index, PROPID propID, FILETIME &filetime, bool &filetimeIsDefined);
|
|
HRESULT GetUnpackSize();
|
|
@@ -138,6 +142,7 @@ public:
|
|
const UStringVector &removePathParts,
|
|
UInt64 packSize);
|
|
|
|
+ bool CreateSymLinks();
|
|
};
|
|
|
|
#endif
|
|
--- a/CPP/7zip/UI/Common/Extract.cpp
|
|
+++ b/CPP/7zip/UI/Common/Extract.cpp
|
|
@@ -96,6 +96,9 @@ static HRESULT DecompressArchive(
|
|
else
|
|
result = archive->Extract(&realIndices.Front(), realIndices.Size(), testMode, extractCallbackSpec);
|
|
|
|
+ if (result == S_OK && !extractCallbackSpec->CreateSymLinks())
|
|
+ result = E_FAIL;
|
|
+
|
|
return callback->ExtractResult(result);
|
|
}
|
|
|
|
--- a/CPP/Windows/FileDir.cpp
|
|
+++ b/CPP/Windows/FileDir.cpp
|
|
@@ -453,9 +453,10 @@ bool SetDirTime(LPCWSTR fileName, const
|
|
}
|
|
|
|
#ifndef _UNICODE
|
|
-bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes)
|
|
+bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes,
|
|
+ CObjectVector<CDelayedSymLink> *delayedSymLinks)
|
|
{
|
|
- return MySetFileAttributes(UnicodeStringToMultiByte(fileName, CP_ACP), fileAttributes);
|
|
+ return MySetFileAttributes(UnicodeStringToMultiByte(fileName, CP_ACP), fileAttributes, delayedSymLinks);
|
|
}
|
|
|
|
bool MyRemoveDirectory(LPCWSTR pathName)
|
|
@@ -488,7 +489,8 @@ static int convert_to_symlink(const char
|
|
return -1;
|
|
}
|
|
|
|
-bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes)
|
|
+bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes,
|
|
+ CObjectVector<CDelayedSymLink> *delayedSymLinks)
|
|
{
|
|
if (!fileName) {
|
|
SetLastError(ERROR_PATH_NOT_FOUND);
|
|
@@ -520,7 +522,9 @@ bool MySetFileAttributes(LPCTSTR fileNam
|
|
stat_info.st_mode = fileAttributes >> 16;
|
|
#ifdef ENV_HAVE_LSTAT
|
|
if (S_ISLNK(stat_info.st_mode)) {
|
|
- if ( convert_to_symlink(name) != 0) {
|
|
+ if (delayedSymLinks)
|
|
+ delayedSymLinks->Add(CDelayedSymLink(name));
|
|
+ else if ( convert_to_symlink(name) != 0) {
|
|
TRACEN((printf("MySetFileAttributes(%s,%d) : false-3\n",name,fileAttributes)))
|
|
return false;
|
|
}
|
|
@@ -924,4 +928,41 @@ bool CTempDirectory::Create(LPCTSTR pref
|
|
}
|
|
|
|
|
|
+#ifdef ENV_UNIX
|
|
+
|
|
+CDelayedSymLink::CDelayedSymLink(LPCSTR source)
|
|
+ : _source(source)
|
|
+{
|
|
+ struct stat st;
|
|
+
|
|
+ if (lstat(_source, &st) == 0) {
|
|
+ _dev = st.st_dev;
|
|
+ _ino = st.st_ino;
|
|
+ } else {
|
|
+ _dev = 0;
|
|
+ }
|
|
+}
|
|
+
|
|
+bool CDelayedSymLink::Create()
|
|
+{
|
|
+ struct stat st;
|
|
+
|
|
+ if (_dev == 0) {
|
|
+ errno = EPERM;
|
|
+ return false;
|
|
+ }
|
|
+ if (lstat(_source, &st) != 0)
|
|
+ return false;
|
|
+ if (_dev != st.st_dev || _ino != st.st_ino) {
|
|
+ // Placeholder file has been overwritten or moved by another
|
|
+ // symbolic link creation
|
|
+ errno = EPERM;
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ return convert_to_symlink(_source) == 0;
|
|
+}
|
|
+
|
|
+#endif // ENV_UNIX
|
|
+
|
|
}}}
|
|
--- a/CPP/Windows/FileDir.h
|
|
+++ b/CPP/Windows/FileDir.h
|
|
@@ -4,6 +4,7 @@
|
|
#define __WINDOWS_FILEDIR_H
|
|
|
|
#include "../Common/MyString.h"
|
|
+#include "../Common/MyVector.h"
|
|
#include "Defs.h"
|
|
|
|
/* GetFullPathName for 7zAES.cpp */
|
|
@@ -13,11 +14,15 @@ namespace NWindows {
|
|
namespace NFile {
|
|
namespace NDirectory {
|
|
|
|
+class CDelayedSymLink;
|
|
+
|
|
bool SetDirTime(LPCWSTR fileName, const FILETIME *creationTime, const FILETIME *lastAccessTime, const FILETIME *lastWriteTime);
|
|
|
|
-bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes);
|
|
+bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes,
|
|
+ CObjectVector<CDelayedSymLink> *delayedSymLinks = 0);
|
|
#ifndef _UNICODE
|
|
-bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes);
|
|
+bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes,
|
|
+ CObjectVector<CDelayedSymLink> *delayedSymLinks = 0);
|
|
#endif
|
|
|
|
bool MyMoveFile(LPCTSTR existFileName, LPCTSTR newFileName);
|
|
@@ -80,6 +85,31 @@ public:
|
|
bool Remove();
|
|
};
|
|
|
|
+// Symbolic links must be created last so that they can't be used to
|
|
+// create or overwrite files above the extraction directory.
|
|
+class CDelayedSymLink
|
|
+{
|
|
+#ifdef ENV_UNIX
|
|
+ // Where the symlink should be created. The target is specified in
|
|
+ // the placeholder file.
|
|
+ AString _source;
|
|
+
|
|
+ // Device and inode of the placeholder file. Before creating the
|
|
+ // symlink, we must check that these haven't been changed by creation
|
|
+ // of another symlink.
|
|
+ dev_t _dev;
|
|
+ ino_t _ino;
|
|
+
|
|
+public:
|
|
+ explicit CDelayedSymLink(LPCSTR source);
|
|
+ bool Create();
|
|
+#else // !ENV_UNIX
|
|
+public:
|
|
+ CDelayedSymLink(LPCSTR source) {}
|
|
+ bool Create() { return true; }
|
|
+#endif // ENV_UNIX
|
|
+};
|
|
+
|
|
#ifdef _UNICODE
|
|
typedef CTempFile CTempFileW;
|
|
#endif
|