1 This patch fixes CVE-2015-1038, filed upstream as |
|
2 |
|
3 http://sourceforge.net/p/p7zip/bugs/147/ |
|
4 |
|
5 The patch contents come from |
|
6 |
|
7 http://sourceforge.net/p/p7zip/bugs/_discuss/thread/17901103/2f9c/attachment/CVE-2015-1038.patch |
|
8 |
|
9 This will presumably be fixed upstream at some point after 9.38.1. |
|
10 |
|
11 ====================================================================== |
|
12 |
|
13 Author: Ben Hutchings <[email protected]> |
|
14 Date: Tue, 19 May 2015 02:38:40 +0100 |
|
15 Description: Delay creation of symlinks to prevent arbitrary file writes (CVE-2015-1038) |
|
16 Bug-Debian: https://bugs.debian.org/774660 |
|
17 |
|
18 Alexander Cherepanov discovered that 7zip is susceptible to a |
|
19 directory traversal vulnerability. While extracting an archive, it |
|
20 will extract symlinks and then follow them if they are referenced in |
|
21 further entries. This can be exploited by a rogue archive to write |
|
22 files outside the current directory. |
|
23 |
|
24 We have to create placeholder files (which we already do) and delay |
|
25 creating symlinks until the end of extraction. |
|
26 |
|
27 Due to the possibility of anti-items (deletions) in the archive, it is |
|
28 possible for placeholders to be deleted and replaced before we create |
|
29 the symlinks. It's not clear that this can be used for mischief, but |
|
30 GNU tar guards against similar problems by checking that the placeholder |
|
31 still exists and is the same inode. XXX It also checks 'birth time' but |
|
32 this isn't portable. We can probably get away with comparing ctime |
|
33 since we don't support hard links. |
|
34 |
|
35 --- a/CPP/7zip/UI/Agent/Agent.cpp |
|
36 +++ b/CPP/7zip/UI/Agent/Agent.cpp |
|
37 @@ -424,6 +424,8 @@ STDMETHODIMP CAgentFolder::Extract(const |
|
38 CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec; |
|
39 UStringVector pathParts; |
|
40 CProxyFolder *currentProxyFolder = _proxyFolderItem; |
|
41 + HRESULT res; |
|
42 + |
|
43 while (currentProxyFolder->Parent) |
|
44 { |
|
45 pathParts.Insert(0, currentProxyFolder->Name); |
|
46 @@ -445,8 +447,11 @@ STDMETHODIMP CAgentFolder::Extract(const |
|
47 (UInt64)(Int64)-1); |
|
48 CUIntVector realIndices; |
|
49 GetRealIndices(indices, numItems, realIndices); |
|
50 - return _agentSpec->GetArchive()->Extract(&realIndices.Front(), |
|
51 + res = _agentSpec->GetArchive()->Extract(&realIndices.Front(), |
|
52 realIndices.Size(), testMode, extractCallback); |
|
53 + if (res == S_OK && !extractCallbackSpec->CreateSymLinks()) |
|
54 + res = E_FAIL; |
|
55 + return res; |
|
56 COM_TRY_END |
|
57 } |
|
58 |
|
59 --- a/CPP/7zip/UI/Agent/ArchiveFolder.cpp |
|
60 +++ b/CPP/7zip/UI/Agent/ArchiveFolder.cpp |
|
61 @@ -20,6 +20,8 @@ STDMETHODIMP CAgentFolder::CopyTo(const |
|
62 CMyComPtr<IArchiveExtractCallback> extractCallback = extractCallbackSpec; |
|
63 UStringVector pathParts; |
|
64 CProxyFolder *currentProxyFolder = _proxyFolderItem; |
|
65 + HRESULT res; |
|
66 + |
|
67 while (currentProxyFolder->Parent) |
|
68 { |
|
69 pathParts.Insert(0, currentProxyFolder->Name); |
|
70 @@ -46,8 +48,11 @@ STDMETHODIMP CAgentFolder::CopyTo(const |
|
71 (UInt64)(Int64)-1); |
|
72 CUIntVector realIndices; |
|
73 GetRealIndices(indices, numItems, realIndices); |
|
74 - return _agentSpec->GetArchive()->Extract(&realIndices.Front(), |
|
75 + res = _agentSpec->GetArchive()->Extract(&realIndices.Front(), |
|
76 realIndices.Size(), BoolToInt(false), extractCallback); |
|
77 + if (res == S_OK && !extractCallbackSpec->CreateSymLinks()) |
|
78 + res = E_FAIL; |
|
79 + return res; |
|
80 COM_TRY_END |
|
81 } |
|
82 |
|
83 --- a/CPP/7zip/UI/Client7z/Client7z.cpp |
|
84 +++ b/CPP/7zip/UI/Client7z/Client7z.cpp |
|
85 @@ -197,8 +197,11 @@ private: |
|
86 COutFileStream *_outFileStreamSpec; |
|
87 CMyComPtr<ISequentialOutStream> _outFileStream; |
|
88 |
|
89 + CObjectVector<NWindows::NFile::NDirectory::CDelayedSymLink> _delayedSymLinks; |
|
90 + |
|
91 public: |
|
92 void Init(IInArchive *archiveHandler, const UString &directoryPath); |
|
93 + bool CreateSymLinks(); |
|
94 |
|
95 UInt64 NumErrors; |
|
96 bool PasswordIsDefined; |
|
97 @@ -392,11 +395,22 @@ STDMETHODIMP CArchiveExtractCallback::Se |
|
98 } |
|
99 _outFileStream.Release(); |
|
100 if (_extractMode && _processedFileInfo.AttribDefined) |
|
101 - NFile::NDirectory::MySetFileAttributes(_diskFilePath, _processedFileInfo.Attrib); |
|
102 + NFile::NDirectory::MySetFileAttributes(_diskFilePath, _processedFileInfo.Attrib, &_delayedSymLinks); |
|
103 PrintNewLine(); |
|
104 return S_OK; |
|
105 } |
|
106 |
|
107 +bool CArchiveExtractCallback::CreateSymLinks() |
|
108 +{ |
|
109 + bool success = true; |
|
110 + |
|
111 + for (int i = 0; i != _delayedSymLinks.Size(); ++i) |
|
112 + success &= _delayedSymLinks[i].Create(); |
|
113 + |
|
114 + _delayedSymLinks.Clear(); |
|
115 + |
|
116 + return success; |
|
117 +} |
|
118 |
|
119 STDMETHODIMP CArchiveExtractCallback::CryptoGetTextPassword(BSTR *password) |
|
120 { |
|
121 --- a/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp |
|
122 +++ b/CPP/7zip/UI/Common/ArchiveExtractCallback.cpp |
|
123 @@ -453,12 +453,24 @@ STDMETHODIMP CArchiveExtractCallback::Se |
|
124 NumFiles++; |
|
125 |
|
126 if (_extractMode && _fi.AttribDefined) |
|
127 - NFile::NDirectory::MySetFileAttributes(_diskFilePath, _fi.Attrib); |
|
128 + NFile::NDirectory::MySetFileAttributes(_diskFilePath, _fi.Attrib, &_delayedSymLinks); |
|
129 RINOK(_extractCallback2->SetOperationResult(operationResult, _encrypted)); |
|
130 return S_OK; |
|
131 COM_TRY_END |
|
132 } |
|
133 |
|
134 +bool CArchiveExtractCallback::CreateSymLinks() |
|
135 +{ |
|
136 + bool success = true; |
|
137 + |
|
138 + for (int i = 0; i != _delayedSymLinks.Size(); ++i) |
|
139 + success &= _delayedSymLinks[i].Create(); |
|
140 + |
|
141 + _delayedSymLinks.Clear(); |
|
142 + |
|
143 + return success; |
|
144 +} |
|
145 + |
|
146 /* |
|
147 STDMETHODIMP CArchiveExtractCallback::GetInStream( |
|
148 const wchar_t *name, ISequentialInStream **inStream) |
|
149 --- a/CPP/7zip/UI/Common/ArchiveExtractCallback.h |
|
150 +++ b/CPP/7zip/UI/Common/ArchiveExtractCallback.h |
|
151 @@ -6,6 +6,8 @@ |
|
152 #include "Common/MyCom.h" |
|
153 #include "Common/Wildcard.h" |
|
154 |
|
155 +#include "Windows/FileDir.h" |
|
156 + |
|
157 #include "../../IPassword.h" |
|
158 |
|
159 #include "../../Common/FileStreams.h" |
|
160 @@ -83,6 +85,8 @@ class CArchiveExtractCallback: |
|
161 UInt64 _packTotal; |
|
162 UInt64 _unpTotal; |
|
163 |
|
164 + CObjectVector<NWindows::NFile::NDirectory::CDelayedSymLink> _delayedSymLinks; |
|
165 + |
|
166 void CreateComplexDirectory(const UStringVector &dirPathParts, UString &fullPath); |
|
167 HRESULT GetTime(int index, PROPID propID, FILETIME &filetime, bool &filetimeIsDefined); |
|
168 HRESULT GetUnpackSize(); |
|
169 @@ -138,6 +142,7 @@ public: |
|
170 const UStringVector &removePathParts, |
|
171 UInt64 packSize); |
|
172 |
|
173 + bool CreateSymLinks(); |
|
174 }; |
|
175 |
|
176 #endif |
|
177 --- a/CPP/7zip/UI/Common/Extract.cpp |
|
178 +++ b/CPP/7zip/UI/Common/Extract.cpp |
|
179 @@ -96,6 +96,9 @@ static HRESULT DecompressArchive( |
|
180 else |
|
181 result = archive->Extract(&realIndices.Front(), realIndices.Size(), testMode, extractCallbackSpec); |
|
182 |
|
183 + if (result == S_OK && !extractCallbackSpec->CreateSymLinks()) |
|
184 + result = E_FAIL; |
|
185 + |
|
186 return callback->ExtractResult(result); |
|
187 } |
|
188 |
|
189 --- a/CPP/Windows/FileDir.cpp |
|
190 +++ b/CPP/Windows/FileDir.cpp |
|
191 @@ -453,9 +453,10 @@ bool SetDirTime(LPCWSTR fileName, const |
|
192 } |
|
193 |
|
194 #ifndef _UNICODE |
|
195 -bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes) |
|
196 +bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes, |
|
197 + CObjectVector<CDelayedSymLink> *delayedSymLinks) |
|
198 { |
|
199 - return MySetFileAttributes(UnicodeStringToMultiByte(fileName, CP_ACP), fileAttributes); |
|
200 + return MySetFileAttributes(UnicodeStringToMultiByte(fileName, CP_ACP), fileAttributes, delayedSymLinks); |
|
201 } |
|
202 |
|
203 bool MyRemoveDirectory(LPCWSTR pathName) |
|
204 @@ -488,7 +489,8 @@ static int convert_to_symlink(const char |
|
205 return -1; |
|
206 } |
|
207 |
|
208 -bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes) |
|
209 +bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes, |
|
210 + CObjectVector<CDelayedSymLink> *delayedSymLinks) |
|
211 { |
|
212 if (!fileName) { |
|
213 SetLastError(ERROR_PATH_NOT_FOUND); |
|
214 @@ -520,7 +522,9 @@ bool MySetFileAttributes(LPCTSTR fileNam |
|
215 stat_info.st_mode = fileAttributes >> 16; |
|
216 #ifdef ENV_HAVE_LSTAT |
|
217 if (S_ISLNK(stat_info.st_mode)) { |
|
218 - if ( convert_to_symlink(name) != 0) { |
|
219 + if (delayedSymLinks) |
|
220 + delayedSymLinks->Add(CDelayedSymLink(name)); |
|
221 + else if ( convert_to_symlink(name) != 0) { |
|
222 TRACEN((printf("MySetFileAttributes(%s,%d) : false-3\n",name,fileAttributes))) |
|
223 return false; |
|
224 } |
|
225 @@ -924,4 +928,41 @@ bool CTempDirectory::Create(LPCTSTR pref |
|
226 } |
|
227 |
|
228 |
|
229 +#ifdef ENV_UNIX |
|
230 + |
|
231 +CDelayedSymLink::CDelayedSymLink(LPCSTR source) |
|
232 + : _source(source) |
|
233 +{ |
|
234 + struct stat st; |
|
235 + |
|
236 + if (lstat(_source, &st) == 0) { |
|
237 + _dev = st.st_dev; |
|
238 + _ino = st.st_ino; |
|
239 + } else { |
|
240 + _dev = 0; |
|
241 + } |
|
242 +} |
|
243 + |
|
244 +bool CDelayedSymLink::Create() |
|
245 +{ |
|
246 + struct stat st; |
|
247 + |
|
248 + if (_dev == 0) { |
|
249 + errno = EPERM; |
|
250 + return false; |
|
251 + } |
|
252 + if (lstat(_source, &st) != 0) |
|
253 + return false; |
|
254 + if (_dev != st.st_dev || _ino != st.st_ino) { |
|
255 + // Placeholder file has been overwritten or moved by another |
|
256 + // symbolic link creation |
|
257 + errno = EPERM; |
|
258 + return false; |
|
259 + } |
|
260 + |
|
261 + return convert_to_symlink(_source) == 0; |
|
262 +} |
|
263 + |
|
264 +#endif // ENV_UNIX |
|
265 + |
|
266 }}} |
|
267 --- a/CPP/Windows/FileDir.h |
|
268 +++ b/CPP/Windows/FileDir.h |
|
269 @@ -4,6 +4,7 @@ |
|
270 #define __WINDOWS_FILEDIR_H |
|
271 |
|
272 #include "../Common/MyString.h" |
|
273 +#include "../Common/MyVector.h" |
|
274 #include "Defs.h" |
|
275 |
|
276 /* GetFullPathName for 7zAES.cpp */ |
|
277 @@ -13,11 +14,15 @@ namespace NWindows { |
|
278 namespace NFile { |
|
279 namespace NDirectory { |
|
280 |
|
281 +class CDelayedSymLink; |
|
282 + |
|
283 bool SetDirTime(LPCWSTR fileName, const FILETIME *creationTime, const FILETIME *lastAccessTime, const FILETIME *lastWriteTime); |
|
284 |
|
285 -bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes); |
|
286 +bool MySetFileAttributes(LPCTSTR fileName, DWORD fileAttributes, |
|
287 + CObjectVector<CDelayedSymLink> *delayedSymLinks = 0); |
|
288 #ifndef _UNICODE |
|
289 -bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes); |
|
290 +bool MySetFileAttributes(LPCWSTR fileName, DWORD fileAttributes, |
|
291 + CObjectVector<CDelayedSymLink> *delayedSymLinks = 0); |
|
292 #endif |
|
293 |
|
294 bool MyMoveFile(LPCTSTR existFileName, LPCTSTR newFileName); |
|
295 @@ -80,6 +85,31 @@ public: |
|
296 bool Remove(); |
|
297 }; |
|
298 |
|
299 +// Symbolic links must be created last so that they can't be used to |
|
300 +// create or overwrite files above the extraction directory. |
|
301 +class CDelayedSymLink |
|
302 +{ |
|
303 +#ifdef ENV_UNIX |
|
304 + // Where the symlink should be created. The target is specified in |
|
305 + // the placeholder file. |
|
306 + AString _source; |
|
307 + |
|
308 + // Device and inode of the placeholder file. Before creating the |
|
309 + // symlink, we must check that these haven't been changed by creation |
|
310 + // of another symlink. |
|
311 + dev_t _dev; |
|
312 + ino_t _ino; |
|
313 + |
|
314 +public: |
|
315 + explicit CDelayedSymLink(LPCSTR source); |
|
316 + bool Create(); |
|
317 +#else // !ENV_UNIX |
|
318 +public: |
|
319 + CDelayedSymLink(LPCSTR source) {} |
|
320 + bool Create() { return true; } |
|
321 +#endif // ENV_UNIX |
|
322 +}; |
|
323 + |
|
324 #ifdef _UNICODE |
|
325 typedef CTempFile CTempFileW; |
|
326 #endif |
|