Skip to content

Commit 0979ba8

Browse files
harden plugin unzipping to zip-slip attacks
1 parent 17ea124 commit 0979ba8

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

Diff for: src/Misc/Utility.cpp

+39
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/************************************************************************
22
**
3+
** Copyright (C) 2019 Kevin B. Hendricks, Stratford, Ontario Canada
34
** Copyright (C) 2009, 2010, 2011 Strahinja Markovic <strahinja.markovic@gmail.com>
45
**
56
** This file is part of Sigil.
@@ -715,6 +716,44 @@ bool Utility::UnZip(const QString &zippath, const QString &destpath)
715716

716717
// If there is no file name then we can't do anything with it.
717718
if (!qfile_name.isEmpty()) {
719+
720+
// for security reasons against maliciously crafted zip archives
721+
// we need the file path to always be inside the target folder
722+
// and not outside, so we will remove all illegal backslashes
723+
// and all relative upward paths segments "/../" from the zip's local
724+
// file name/path before prepending the target folder to create
725+
// the final path
726+
727+
QString original_path = qfile_name;
728+
bool evil_or_corrupt_epub = false;
729+
730+
if (qfile_name.contains("\\")) evil_or_corrupt_epub = true;
731+
qfile_name = "/" + qfile_name.replace("\\","");
732+
733+
if (qfile_name.contains("/../")) evil_or_corrupt_epub = true;
734+
qfile_name = qfile_name.replace("/../","/");
735+
736+
while(qfile_name.startsWith("/")) {
737+
qfile_name = qfile_name.remove(0,1);
738+
}
739+
740+
if (cp437_file_name.contains("\\")) evil_or_corrupt_epub = true;
741+
cp437_file_name = "/" + cp437_file_name.replace("\\","");
742+
743+
if (cp437_file_name.contains("/../")) evil_or_corrupt_epub = true;
744+
cp437_file_name = cp437_file_name.replace("/../","/");
745+
746+
while(cp437_file_name.startsWith("/")) {
747+
cp437_file_name = cp437_file_name.remove(0,1);
748+
}
749+
750+
if (evil_or_corrupt_epub) {
751+
unzCloseCurrentFile(zfile);
752+
unzClose(zfile);
753+
// throw (UNZIPLoadParseError(QString(QObject::tr("Possible evil or corrupt zip file name: %1")).arg(original_path).toStdString()));
754+
return false;
755+
}
756+
718757
// We use the dir object to create the path in the temporary directory.
719758
// Unfortunately, we need a dir ojbect to do this as it's not a static function.
720759
// Full file path in the temporary directory.

Diff for: src/sigil_exception.h

+10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/************************************************************************
22
**
3+
** Copyright (C) 2019 Kevin B. Hendricks, Stratford, Ontario Canada
34
** Copyright (C) 2015 John Schember <john@nachtimwald.com>
45
** Copyright (C) 2009, 2010, 2011 Strahinja Markovic <strahinja.markovic@gmail.com>
56
**
@@ -132,4 +133,13 @@ class EPUBLoadParseError : public std::runtime_error {
132133
EPUBLoadParseError(const std::string &msg) : std::runtime_error(msg) { };
133134
};
134135

136+
137+
/**
138+
* Thrown for Invalid EPUB errors while loading and parsing content files.
139+
*/
140+
class UNZIPLoadParseError : public std::runtime_error {
141+
public:
142+
UNZIPLoadParseError(const std::string &msg) : std::runtime_error(msg) { };
143+
};
144+
135145
#endif // SG_EXCEPTION_H

0 commit comments

Comments
 (0)