CVE-2019-14452: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Sigil before 0.9.16 is vulnerable to a directory traversal, allowing attackers to write arbitrary files via a ../ (dot dot slash) in a ZIP archive entry that is mishandled during extraction.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.5
Published: 2019-07-31T02:15Z
Repository: https://github.com/Sigil-Ebook/Sigil
Language: C++
Files changed: src/Importers/ImportEPUB.cpp
Commit date: 2019-06-27 11:47:42 -0400
Commit message: further harden against malicious epubs and produce error message

Diff: diff --git a/src/Importers/ImportEPUB.cpp b/src/Importers/ImportEPUB.cpp
index 501f49f8..2f5c25f4 100644
--- a/src/Importers/ImportEPUB.cpp
+++ b/src/Importers/ImportEPUB.cpp
@@ -423,16 +423,45 @@ void ImportEPUB::ExtractContainer()
             }
 
             // If there is no file name then we can't do anything with it.
             if (!qfile_name.isEmpty()) {
 
-	        // for security reasons we need the file path to always be inside the 
-                // target folder and not outside, so we will remove all relative upward 
-                // paths segments ".." from the file path before prepending the target 
-                // folder to create the final target path
-	        qfile_name = qfile_name.replace("../","");
-                cp437_file_name = cp437_file_name.replace("../","");
+	        // for security reasons against maliciously crafted zip archives
+                // we need the file path to always be inside the target folder 
+                // and not outside, so we will remove all illegal backslashes
+                // and all relative upward paths segments "/../" from the zip's local 
+                // file name/path before prepending the target folder to create 
+                // the final path
+
+	        QString original_path = qfile_name;
+	        bool evil_or_corrupt_epub = false;
+
+                if (qfile_name.contains("\\")) evil_or_corrupt_epub = true; 
+	        qfile_name = "/" + qfile_name.replace("\\","");
+
+                if (qfile_name.contains("/../")) evil_or_corrupt_epub = true;
+	        qfile_name = qfile_name.replace("/../","/");
+
+                while(qfile_name.startsWith("/")) { 
+		    qfile_name = qfile_name.remove(0,1);
+		}
+
+                if (cp437_file_name.contains("\\")) evil_or_corrupt_epub = true; 
+                cp437_file_name = "/" + cp437_file_name.replace("\\","");
+
+                if (cp437_file_name.contains("/../")) evil_or_corrupt_epub = true;
+	        cp437_file_name = cp437_file_name.replace("/../","/");
+
+                while(cp437_file_name.startsWith("/")) { 
+		    cp437_file_name = cp437_file_name.remove(0,1);
+		}
+
+                if (evil_or_corrupt_epub) {
+                    unzCloseCurrentFile(zfile);
+                    unzClose(zfile);
+                    throw (EPUBLoadParseError(QString(QObject::tr("Possible evil or corrupt epub file name: %1")).arg(original_path).toStdString()));
+                }
 
                 // We use the dir object to create the path in the temporary directory.
                 // Unfortunately, we need a dir ojbect to do this as it's not a static function.
                 QDir dir(m_ExtractedFolderPath);
                 // Full file path in the temporary directory.

Vulnerable code: 	        // for security reasons we need the file path to always be inside the 
                // target folder and not outside, so we will remove all relative upward 
                // paths segments ".." from the file path before prepending the target 
                // folder to create the final target path
	        qfile_name = qfile_name.replace("../","");
                cp437_file_name = cp437_file_name.replace("../","");
Fixed code: 	        // for security reasons against maliciously crafted zip archives
                // we need the file path to always be inside the target folder 
                // and not outside, so we will remove all illegal backslashes
                // and all relative upward paths segments "/../" from the zip's local 
                // file name/path before prepending the target folder to create 
                // the final path

	        QString original_path = qfile_name;
	        bool evil_or_corrupt_epub = false;

                if (qfile_name.contains("\\")) evil_or_corrupt_epub = true; 
	        qfile_name = "/" + qfile_name.replace("\\","");

                if (qfile_name.contains("/../")) evil_or_corrupt_epub = true;
	        qfile_name = qfile_name.replace("/../","/");

                while(qfile_name.startsWith("/")) { 
		    qfile_name = qfile_name.remove(0,1);
		}

                if (cp437_file_name.contains("\\")) evil_or_corrupt_epub = true; 
                cp437_file_name = "/" + cp437_file_name.replace("\\","");

                if (cp437_file_name.contains("/../")) evil_or_corrupt_epub = true;
	        cp437_file_name = cp437_file_name.replace("/../","/");

                while(cp437_file_name.startsWith("/")) { 
		    cp437_file_name = cp437_file_name.remove(0,1);
		}

                if (evil_or_corrupt_epub) {
                    unzCloseCurrentFile(zfile);
                    unzClose(zfile);
                    throw (EPUBLoadParseError(QString(QObject::tr("Possible evil or corrupt epub file name: %1")).arg(original_path).toStdString()));
                }