summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSylvain Beucler <beuc@beuc.net>2018-06-05 23:07:56 +0200
committerSylvain Beucler <beuc@beuc.net>2018-06-05 23:07:56 +0200
commit40cc957f52e772f45125126439ba9333cf2d2998 (patch)
tree3274fd702c2d418b448419990f55aca25b0358d8
parentc14fd8aeeeadca0f49388f17eaf21b49bb754b76 (diff)
downloaddfarc-40cc957f52e772f45125126439ba9333cf2d2998.tar.gz
Fix directory traversal in D-Mod extractor (CVE-2018-0496)3.12
-rw-r--r--src/InstallVerifyFrame.cpp22
-rw-r--r--src/Tar.cpp81
-rw-r--r--src/Tar.hpp4
3 files changed, 85 insertions, 22 deletions
diff --git a/src/InstallVerifyFrame.cpp b/src/InstallVerifyFrame.cpp
index 64964e1..494ba53 100644
--- a/src/InstallVerifyFrame.cpp
+++ b/src/InstallVerifyFrame.cpp
@@ -3,7 +3,7 @@
* Copyright (C) 2004 Andrew Reading
* Copyright (C) 2005, 2006 Dan Walma
- * Copyright (C) 2008 Sylvain Beucler
+ * Copyright (C) 2008, 2018 Sylvain Beucler
* This file is part of GNU FreeDink
@@ -55,7 +55,10 @@ InstallVerifyFrame::InstallVerifyFrame(const wxString& lDmodFilePath)
{
// Prepare the tar file for reading
Tar lTar(mTarFilePath);
- lTar.ReadHeaders();
+ if (lTar.ReadHeaders() == 1) {
+ this->EndModal(wxID_CANCEL);
+ return;
+ }
// Get and display the dmod description
wxString lDmodDescription = lTar.getmDmodDescription();
@@ -122,7 +125,20 @@ void InstallVerifyFrame::onInstall(wxCommandEvent &Event)
destdir = mConfig->mDModDir;
Tar lTar(mTarFilePath);
- lTar.ReadHeaders();
+ if (lTar.ReadHeaders() == 1) {
+ this->EndModal(wxID_CANCEL);
+ return;
+ }
+
+ if (wxDirExists(destdir + wxFileName::GetPathSeparator() + lTar.getInstalledDmodDirectory())) {
+ wxString question;
+ question.Printf(_("Directory '%s' already exists. Continue?"), lTar.getInstalledDmodDirectory());
+ int lResult = wxMessageBox(question, _("DFArc - Installing"),
+ wxYES_NO | wxICON_WARNING, this);
+ if (lResult == wxNO)
+ return;
+ }
+
int lError = lTar.Extract(destdir, &lInstallProgress);
if (lError == 0)
{
diff --git a/src/Tar.cpp b/src/Tar.cpp
index b919ae7..8147b8a 100644
--- a/src/Tar.cpp
+++ b/src/Tar.cpp
@@ -3,7 +3,7 @@
* Copyright (C) 2004 Andrew Reading
* Copyright (C) 2005, 2006 Dan Walma
- * Copyright (C) 2008, 2014 Sylvain Beucler
+ * Copyright (C) 2008, 2014, 2018 Sylvain Beucler
* This file is part of GNU FreeDink
@@ -31,6 +31,7 @@
#include <wx/intl.h>
#include <wx/log.h>
#include <wx/filename.h>
+#include <wx/tokenzr.h>
#include <math.h>
#include <ext/stdio_filebuf.h>
@@ -427,7 +428,15 @@ int Tar::ReadHeaders( void )
wxString lPath(lRecord.Name, wxConvUTF8);
if (mInstalledDmodDirectory.Length() == 0)
{
- mInstalledDmodDirectory = lPath.SubString( 0, lPath.Find( '/' ) );
+ // Security: ensure the D-Mod directory is non-empty
+ wxString firstDir = GetFirstDir(lPath);
+ if (firstDir.IsSameAs("", true) || firstDir.IsSameAs("..", true) || firstDir.IsSameAs("dink", true))
+ {
+ wxLogError(_("Error: invalid D-Mod directory. Stopping."));
+ return 1;
+ }
+ mInstalledDmodDirectory = firstDir;
+
lDmodDizPath = mInstalledDmodDirectory + _T("dmod.diz");
lDmodDizPath.LowerCase();
}
@@ -472,10 +481,6 @@ int Tar::Extract(wxString destdir, wxProgressDialog* aProgressDialog)
wxString strBuf;
int lError = 0;
- // Remember current directory
- wxString strCwd = ::wxGetCwd();
-
-
// Open the file here so it doesn't error after changing.
wxFile wx_In(mFilePath, wxFile::read);
@@ -495,8 +500,6 @@ int Tar::Extract(wxString destdir, wxProgressDialog* aProgressDialog)
wxLogFatalError(_("Error: Cannot create directory '%s'. Cannot extract data."), destdir.c_str());
throw;
}
- // Move to the directory.
- ::wxSetWorkingDirectory(destdir);
// Put the data in the directories.
__gnu_cxx::stdio_filebuf<char> filebuf(wx_In.fd(), std::ios::in);
@@ -507,10 +510,6 @@ int Tar::Extract(wxString destdir, wxProgressDialog* aProgressDialog)
}
wx_In.Close();
-
- // We're done. Move back.
- ::wxSetWorkingDirectory(strCwd);
-
return lError;
}
@@ -527,10 +526,6 @@ int Tar::ExtractData(std::istream& aTarStreamIn, wxString destdir, wxProgressDia
aTarStreamIn.seekg(0, std::ios::beg);
lTotalBytes = lEnd - static_cast<unsigned long>(aTarStreamIn.tellg());
- // Move into the extract dir.
- wxString lPreviousWorkingDirectory(::wxGetCwd());
- ::wxSetWorkingDirectory(destdir);
-
// Extract the files.
int ebufsiz = 8192;
char buffer[ebufsiz];
@@ -543,7 +538,16 @@ int Tar::ExtractData(std::istream& aTarStreamIn, wxString destdir, wxProgressDia
/* Attempt convertion from latin-1 if not valid UTF-8 */
lCurrentFilePath = wxString(lCurrentTarRecord.Name, wxConvISO8859_1);
}
- wxString lCurrentDirectory(lCurrentFilePath.substr(0, lCurrentFilePath.find_last_of('/')));
+ // Security: check if archive tries to jump out of destination directory
+ if (IsPathInsecure(lCurrentFilePath))
+ {
+ wxLogError(_("Error: Insecure filename: '%s'. Stopping."), lCurrentFilePath);
+ lError = 1;
+ break;
+ }
+ // Security: ensure full, non-relative path, under destdir/
+ lCurrentFilePath = destdir + wxFileName::GetPathSeparator() + lCurrentFilePath;
+ wxString lCurrentDirectory = lCurrentFilePath.substr(0, lCurrentFilePath.find_last_of("/\\"));
// Odd bad file problem...
if (lCurrentFilePath.compare(_T("\xFF")) == 0) // "ÿ"
@@ -612,7 +616,6 @@ int Tar::ExtractData(std::istream& aTarStreamIn, wxString destdir, wxProgressDia
}
}
aProgressDialog->Update(100, _("Done."));
- ::wxSetWorkingDirectory(lPreviousWorkingDirectory);
return lError;
}
@@ -687,3 +690,45 @@ int Tar::RoundTo512(int n)
return (n - (n % 512)) + 512;
}
}
+
+wxString Tar::GetFirstDir(wxString path) {
+ wxString firstDir = "";
+ wxString previousDir = "";
+ // tokenizer never returns empty strings + distinguish dir// and a/$
+ if (path.EndsWith("/") || path.EndsWith("\\"))
+ path += "dummy";
+ wxStringTokenizer tokenizer(path, "/\\", wxTOKEN_STRTOK);
+ while (tokenizer.HasMoreTokens()) {
+ wxString curDir = tokenizer.GetNextToken();
+ if (curDir == '.')
+ continue;
+ if (previousDir != "") {
+ firstDir = previousDir;
+ break;
+ }
+ previousDir = curDir;
+ }
+ return firstDir;
+}
+
+// Security: check if archive tries to jump out of destination directory
+bool Tar::IsPathInsecure(wxString path) {
+ // Avoid leading slashes (even if we preprend destdir)
+ if (path[0] == '/' || path[0] == '\\')
+ return true;
+ // Avoid ':' since wxFileName::Mkdir silently normalizes
+ // e.g. C:\test1\C:\test2 to C:\test2
+ if (path.Contains(":"))
+ return true;
+ // Ensure all files reside in the same subdirectory
+ if (GetFirstDir(path) != mInstalledDmodDirectory)
+ return true;
+ // Ensure there's no '..' path element
+ wxStringTokenizer tokenizer(path, "/\\");
+ while (tokenizer.HasMoreTokens()) {
+ wxString token = tokenizer.GetNextToken();
+ if (token == "..")
+ return true;
+ }
+ return false;
+}
diff --git a/src/Tar.hpp b/src/Tar.hpp
index f612580..62d7147 100644
--- a/src/Tar.hpp
+++ b/src/Tar.hpp
@@ -3,7 +3,7 @@
* Copyright (C) 2004 Andrew Reading
* Copyright (C) 2005, 2006 Dan Walma
- * Copyright (C) 2008 Sylvain Beucler
+ * Copyright (C) 2008, 2018 Sylvain Beucler
* This file is part of GNU FreeDink
@@ -88,6 +88,8 @@ private:
bool VerifyChecksum(TarHeader *Header);
bool CreateReal(const wxString& aFolderName, double *compression_ratio, wxProgressDialog* aProgressDialog);
int ExtractData(std::istream &f_In, wxString destdir, wxProgressDialog* aProgressDialog);
+ wxString GetFirstDir(wxString path);
+ bool IsPathInsecure(wxString path);
// True if the proper constructor was used. The compression method
// will not screw up if this flag is set to true.