File p7zip-CVE-2015-1038.patch of Package p7zip

Overview Repositories Revisions Requests Users Attributes Meta

File p7zip-CVE-2015-1038.patch of Package p7zip (Revision 6db699d8f3189308702e1418d2b1bb6b)

Currently displaying revision 6db699d8f3189308702e1418d2b1bb6b , Show latest

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-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.

Index: CPP/7zip/UI/Agent/Agent.cpp
===================================================================
--- CPP/7zip/UI/Agent/Agent.cpp.orig	2010-11-07 16:17:17.000000000 +0100
+++ CPP/7zip/UI/Agent/Agent.cpp	2015-06-23 11:25:57.182342870 +0200
@@ -424,6 +424,8 @@
   CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec;
   UStringVector pathParts;
   CProxyFolder *currentProxyFolder = _proxyFolderItem;
+  HRESULT res;
+
   while (currentProxyFolder->Parent)
   {
     pathParts.Insert(0, currentProxyFolder->Name);
@@ -445,8 +447,11 @@
       (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
 }
 
Index: CPP/7zip/UI/Agent/ArchiveFolder.cpp
===================================================================
--- CPP/7zip/UI/Agent/ArchiveFolder.cpp.orig	2009-05-31 10:11:03.000000000 +0200
+++ CPP/7zip/UI/Agent/ArchiveFolder.cpp	2015-06-23 11:25:57.183342883 +0200
@@ -20,6 +20,8 @@
   CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec;
   UStringVector pathParts;
   CProxyFolder *currentProxyFolder = _proxyFolderItem;
+  HRESULT res;
+
   while (currentProxyFolder->Parent)
   {
     pathParts.Insert(0, currentProxyFolder->Name);
@@ -46,8 +48,11 @@
       (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
 }
 
Index: CPP/7zip/UI/Client7z/Client7z.cpp
===================================================================
--- CPP/7zip/UI/Client7z/Client7z.cpp.orig	2009-12-21 12:46:32.000000000 +0100
+++ CPP/7zip/UI/Client7z/Client7z.cpp	2015-06-23 11:25:57.183342883 +0200
@@ -197,8 +197,11 @@
   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 @@
   }
   _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)
 {
Index: CPP/7zip/UI/Common/ArchiveExtractCallback.cpp
===================================================================
--- CPP/7zip/UI/Common/ArchiveExtractCallback.cpp.orig	2009-05-31 10:11:04.000000000 +0200
+++ CPP/7zip/UI/Common/ArchiveExtractCallback.cpp	2015-06-23 11:25:57.183342883 +0200
@@ -453,12 +453,24 @@
     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)
Index: CPP/7zip/UI/Common/ArchiveExtractCallback.h
===================================================================
--- CPP/7zip/UI/Common/ArchiveExtractCallback.h.orig	2009-05-31 10:10:02.000000000 +0200
+++ CPP/7zip/UI/Common/ArchiveExtractCallback.h	2015-06-23 11:25:57.183342883 +0200
@@ -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 @@
   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 @@
       const UStringVector &removePathParts,
       UInt64 packSize);
 
+  bool CreateSymLinks();
 };
 
 #endif
Index: CPP/7zip/UI/Common/Extract.cpp
===================================================================
--- CPP/7zip/UI/Common/Extract.cpp.orig	2010-11-07 16:17:19.000000000 +0100
+++ CPP/7zip/UI/Common/Extract.cpp	2015-06-23 11:25:57.184342896 +0200
@@ -96,6 +96,9 @@
   else
     result = archive->Extract(&realIndices.Front(), realIndices.Size(), testMode, extractCallbackSpec);
 
+  if (result == S_OK && !extractCallbackSpec->CreateSymLinks())
+    result = E_FAIL;
+
   return callback->ExtractResult(result);
 }
 
Index: CPP/Windows/FileDir.cpp
===================================================================
--- CPP/Windows/FileDir.cpp.orig	2011-01-11 21:55:15.000000000 +0100
+++ CPP/Windows/FileDir.cpp	2015-06-23 11:25:57.184342896 +0200
@@ -453,9 +453,10 @@
 }
 
 #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 @@
   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 @@
      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 @@
 }
 
 
+#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
+
 }}}
Index: CPP/Windows/FileDir.h
===================================================================
--- CPP/Windows/FileDir.h.orig	2010-04-03 18:47:36.000000000 +0200
+++ CPP/Windows/FileDir.h	2015-06-23 11:25:57.184342896 +0200
@@ -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 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 @@
   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

Places

File p7zip-CVE-2015-1038.patch of Package p7zip (Revision 6db699d8f3189308702e1418d2b1bb6b)

Places