parent
52fba4ffe0
commit
43d1d572b0
@ -1,290 +0,0 @@
|
|||||||
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.
|
|
||||||
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/7zip/UI/Agent/Agent.cpp p7zip_15.09/CPP/7zip/UI/Agent/Agent.cpp
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/UI/Agent/Agent.cpp 2015-09-17 20:02:35.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/UI/Agent/Agent.cpp 2015-12-03 02:22:47.073724194 +0000
|
|
||||||
@@ -1515,7 +1515,7 @@ STDMETHODIMP CAgentFolder::Extract(const
|
|
||||||
HRESULT result = _agentSpec->GetArchive()->Extract(&realIndices.Front(),
|
|
||||||
realIndices.Size(), testMode, extractCallback);
|
|
||||||
if (result == S_OK)
|
|
||||||
- result = extractCallbackSpec->SetDirsTimes();
|
|
||||||
+ result = extractCallbackSpec->SetFinalAttribs();
|
|
||||||
return result;
|
|
||||||
COM_TRY_END
|
|
||||||
}
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/7zip/UI/Client7z/Client7z.cpp p7zip_15.09/CPP/7zip/UI/Client7z/Client7z.cpp
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/UI/Client7z/Client7z.cpp 2015-10-17 15:52:30.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/UI/Client7z/Client7z.cpp 2015-12-03 02:22:47.073724194 +0000
|
|
||||||
@@ -230,8 +230,11 @@ private:
|
|
||||||
COutFileStream *_outFileStreamSpec;
|
|
||||||
CMyComPtr<ISequentialOutStream> _outFileStream;
|
|
||||||
|
|
||||||
+ CObjectVector<NWindows::NFile::NDir::CDelayedSymLink> _delayedSymLinks;
|
|
||||||
+
|
|
||||||
public:
|
|
||||||
void Init(IInArchive *archiveHandler, const FString &directoryPath);
|
|
||||||
+ HRESULT SetFinalAttribs();
|
|
||||||
|
|
||||||
UInt64 NumErrors;
|
|
||||||
bool PasswordIsDefined;
|
|
||||||
@@ -449,11 +452,23 @@ STDMETHODIMP CArchiveExtractCallback::Se
|
|
||||||
}
|
|
||||||
_outFileStream.Release();
|
|
||||||
if (_extractMode && _processedFileInfo.AttribDefined)
|
|
||||||
- SetFileAttrib(_diskFilePath, _processedFileInfo.Attrib);
|
|
||||||
+ SetFileAttrib(_diskFilePath, _processedFileInfo.Attrib, &_delayedSymLinks);
|
|
||||||
PrintNewLine();
|
|
||||||
return S_OK;
|
|
||||||
}
|
|
||||||
|
|
||||||
+HRESULT CArchiveExtractCallback::SetFinalAttribs()
|
|
||||||
+{
|
|
||||||
+ HRESULT result = S_OK;
|
|
||||||
+
|
|
||||||
+ for (int i = 0; i != _delayedSymLinks.Size(); ++i)
|
|
||||||
+ if (!_delayedSymLinks[i].Create())
|
|
||||||
+ result = E_FAIL;
|
|
||||||
+
|
|
||||||
+ _delayedSymLinks.Clear();
|
|
||||||
+
|
|
||||||
+ return result;
|
|
||||||
+}
|
|
||||||
|
|
||||||
STDMETHODIMP CArchiveExtractCallback::CryptoGetTextPassword(BSTR *password)
|
|
||||||
{
|
|
||||||
@@ -914,6 +929,8 @@ int MY_CDECL main(int numArgs, const cha
|
|
||||||
// extractCallbackSpec->PasswordIsDefined = true;
|
|
||||||
// extractCallbackSpec->Password = L"1";
|
|
||||||
HRESULT result = archive->Extract(NULL, (UInt32)(Int32)(-1), false, extractCallback);
|
|
||||||
+ if (result == S_OK)
|
|
||||||
+ result = extractCallbackSpec->SetFinalAttribs();
|
|
||||||
if (result != S_OK)
|
|
||||||
{
|
|
||||||
PrintError("Extract Error");
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp p7zip_15.09/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp 2015-10-03 09:49:15.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp 2015-12-03 02:24:40.444963545 +0000
|
|
||||||
@@ -1502,7 +1502,7 @@ STDMETHODIMP CArchiveExtractCallback::Se
|
|
||||||
NumFiles++;
|
|
||||||
|
|
||||||
if (!_stdOutMode && _extractMode && _fi.AttribDefined)
|
|
||||||
- SetFileAttrib(_diskFilePath, _fi.Attrib);
|
|
||||||
+ SetFileAttrib(_diskFilePath, _fi.Attrib, &_delayedSymLinks);
|
|
||||||
|
|
||||||
RINOK(_extractCallback2->SetOperationResult(opRes, BoolToInt(_encrypted)));
|
|
||||||
|
|
||||||
@@ -1584,8 +1584,9 @@ static unsigned GetNumSlashes(const FCha
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
-HRESULT CArchiveExtractCallback::SetDirsTimes()
|
|
||||||
+HRESULT CArchiveExtractCallback::SetFinalAttribs()
|
|
||||||
{
|
|
||||||
+ HRESULT result = S_OK;
|
|
||||||
CRecordVector<CExtrRefSortPair> pairs;
|
|
||||||
pairs.ClearAndSetSize(_extractedFolderPaths.Size());
|
|
||||||
unsigned i;
|
|
||||||
@@ -1622,5 +1623,12 @@ HRESULT CArchiveExtractCallback::SetDirs
|
|
||||||
(WriteATime && ATimeDefined) ? &ATime : NULL,
|
|
||||||
(WriteMTime && MTimeDefined) ? &MTime : (_arc->MTimeDefined ? &_arc->MTime : NULL));
|
|
||||||
}
|
|
||||||
- return S_OK;
|
|
||||||
+
|
|
||||||
+ for (int i = 0; i != _delayedSymLinks.Size(); ++i)
|
|
||||||
+ if (!_delayedSymLinks[i].Create())
|
|
||||||
+ result = E_FAIL;
|
|
||||||
+
|
|
||||||
+ _delayedSymLinks.Clear();
|
|
||||||
+
|
|
||||||
+ return result;
|
|
||||||
}
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/7zip/UI/Common/ArchiveExtractCallback.h p7zip_15.09/CPP/7zip/UI/Common/ArchiveExtractCallback.h
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/UI/Common/ArchiveExtractCallback.h 2015-10-03 11:29:09.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/UI/Common/ArchiveExtractCallback.h 2015-12-03 02:22:47.074724204 +0000
|
|
||||||
@@ -6,6 +6,8 @@
|
|
||||||
#include "../../../Common/MyCom.h"
|
|
||||||
#include "../../../Common/Wildcard.h"
|
|
||||||
|
|
||||||
+#include "../../../Windows/FileDir.h"
|
|
||||||
+
|
|
||||||
#include "../../IPassword.h"
|
|
||||||
|
|
||||||
#include "../../Common/FileStreams.h"
|
|
||||||
@@ -237,6 +239,8 @@ class CArchiveExtractCallback:
|
|
||||||
bool _saclEnabled;
|
|
||||||
#endif
|
|
||||||
|
|
||||||
+ CObjectVector<NWindows::NFile::NDir::CDelayedSymLink> _delayedSymLinks;
|
|
||||||
+
|
|
||||||
void CreateComplexDirectory(const UStringVector &dirPathParts, FString &fullPath);
|
|
||||||
HRESULT GetTime(int index, PROPID propID, FILETIME &filetime, bool &filetimeIsDefined);
|
|
||||||
HRESULT GetUnpackSize();
|
|
||||||
@@ -330,7 +334,7 @@ public:
|
|
||||||
}
|
|
||||||
#endif
|
|
||||||
|
|
||||||
- HRESULT SetDirsTimes();
|
|
||||||
+ HRESULT SetFinalAttribs();
|
|
||||||
};
|
|
||||||
|
|
||||||
bool CensorNode_CheckPath(const NWildcard::CCensorNode &node, const CReadArcItem &item);
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/7zip/UI/Common/Extract.cpp p7zip_15.09/CPP/7zip/UI/Common/Extract.cpp
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/UI/Common/Extract.cpp 2015-09-07 20:47:32.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/UI/Common/Extract.cpp 2015-12-03 02:22:47.075724215 +0000
|
|
||||||
@@ -207,7 +207,7 @@ static HRESULT DecompressArchive(
|
|
||||||
else
|
|
||||||
result = archive->Extract(&realIndices.Front(), realIndices.Size(), testMode, ecs);
|
|
||||||
if (result == S_OK && !options.StdInMode)
|
|
||||||
- result = ecs->SetDirsTimes();
|
|
||||||
+ result = ecs->SetFinalAttribs();
|
|
||||||
return callback->ExtractResult(result);
|
|
||||||
}
|
|
||||||
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/Windows/FileDir.cpp p7zip_15.09/CPP/Windows/FileDir.cpp
|
|
||||||
--- p7zip_15.09.orig/CPP/Windows/FileDir.cpp 2015-10-10 13:37:41.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/Windows/FileDir.cpp 2015-12-03 02:22:47.075724215 +0000
|
|
||||||
@@ -347,7 +347,8 @@ static int convert_to_symlink(const char
|
|
||||||
return -1;
|
|
||||||
}
|
|
||||||
|
|
||||||
-bool SetFileAttrib(CFSTR fileName, DWORD fileAttributes)
|
|
||||||
+bool SetFileAttrib(CFSTR fileName, DWORD fileAttributes,
|
|
||||||
+ CObjectVector<CDelayedSymLink> *delayedSymLinks)
|
|
||||||
{
|
|
||||||
if (!fileName) {
|
|
||||||
SetLastError(ERROR_PATH_NOT_FOUND);
|
|
||||||
@@ -379,7 +380,9 @@ bool SetFileAttrib(CFSTR fileName, DWORD
|
|
||||||
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("SetFileAttrib(%s,%d) : false-3\n",(const char *)name,fileAttributes)))
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
@@ -814,6 +817,43 @@ bool CTempDir::Remove()
|
|
||||||
return !_mustBeDeleted;
|
|
||||||
}
|
|
||||||
|
|
||||||
+#ifdef ENV_UNIX
|
|
||||||
+
|
|
||||||
+CDelayedSymLink::CDelayedSymLink(const char * 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
|
|
||||||
+
|
|
||||||
}}}
|
|
||||||
|
|
||||||
#ifndef _SFX
|
|
||||||
diff -rup p7zip_15.09.orig/CPP/Windows/FileDir.h p7zip_15.09/CPP/Windows/FileDir.h
|
|
||||||
--- p7zip_15.09.orig/CPP/Windows/FileDir.h 2015-06-19 11:52:06.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/Windows/FileDir.h 2015-12-03 02:22:47.075724215 +0000
|
|
||||||
@@ -4,6 +4,7 @@
|
|
||||||
#define __WINDOWS_FILE_DIR_H
|
|
||||||
|
|
||||||
#include "../Common/MyString.h"
|
|
||||||
+#include "../Common/MyVector.h"
|
|
||||||
|
|
||||||
#include "FileIO.h"
|
|
||||||
|
|
||||||
@@ -11,11 +12,14 @@ namespace NWindows {
|
|
||||||
namespace NFile {
|
|
||||||
namespace NDir {
|
|
||||||
|
|
||||||
+class CDelayedSymLink;
|
|
||||||
+
|
|
||||||
bool GetWindowsDir(FString &path);
|
|
||||||
bool GetSystemDir(FString &path);
|
|
||||||
|
|
||||||
bool SetDirTime(CFSTR path, const FILETIME *cTime, const FILETIME *aTime, const FILETIME *mTime);
|
|
||||||
-bool SetFileAttrib(CFSTR path, DWORD attrib);
|
|
||||||
+bool SetFileAttrib(CFSTR path, DWORD attrib,
|
|
||||||
+ CObjectVector<CDelayedSymLink> *delayedSymLinks = 0);
|
|
||||||
bool MyMoveFile(CFSTR existFileName, CFSTR newFileName);
|
|
||||||
|
|
||||||
#ifndef UNDER_CE
|
|
||||||
@@ -76,6 +80,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(const char * source);
|
|
||||||
+ bool Create();
|
|
||||||
+#else // !ENV_UNIX
|
|
||||||
+public:
|
|
||||||
+ CDelayedSymLink(const char * source) {}
|
|
||||||
+ bool Create() { return true; }
|
|
||||||
+#endif // ENV_UNIX
|
|
||||||
+};
|
|
||||||
+
|
|
||||||
#if !defined(UNDER_CE)
|
|
||||||
class CCurrentDirRestorer
|
|
||||||
{
|
|
@ -1,15 +0,0 @@
|
|||||||
diff -rup p7zip_15.09.orig/CPP/7zip/CMAKE/CMakeLists.txt p7zip_15.09/CPP/7zip/CMAKE/CMakeLists.txt
|
|
||||||
--- p7zip_15.09.orig/CPP/7zip/CMAKE/CMakeLists.txt 2015-06-21 20:53:26.000000000 +0100
|
|
||||||
+++ p7zip_15.09/CPP/7zip/CMAKE/CMakeLists.txt 2015-12-21 04:09:58.900036833 +0000
|
|
||||||
@@ -27,9 +27,9 @@ add_subdirectory(7za)
|
|
||||||
|
|
||||||
add_subdirectory(7z_)
|
|
||||||
|
|
||||||
-add_subdirectory(7zG)
|
|
||||||
+#add_subdirectory(7zG)
|
|
||||||
|
|
||||||
-add_subdirectory(7zFM)
|
|
||||||
+#add_subdirectory(7zFM)
|
|
||||||
|
|
||||||
add_subdirectory(7zr)
|
|
||||||
|
|
Loading…
Reference in new issue