Commit 0df59252 authored by Elvis Angelaccio's avatar Elvis Angelaccio

Fix vulnerability to path traversal attacks

Ark was vulnerable to directory traversal attacks because of
missing validation of file paths in the archive.

More details about this attack are available at:
https://github.com/snyk/zip-slip-vulnerability

Job::onEntry() is the only place where we can safely check the path of
every entry in the archive. There shouldn't be a valid reason
to have a "../" in an archive path, so we can just play safe and abort
the LoadJob if we detect such an entry. This makes impossibile to
extract this kind of malicious archives and perform the attack.

Thanks to Albert Astals Cid for suggesting to use QDir::cleanPath()
so that we can still allow loading of legitimate archives that
contain "../" in their paths but still resolve inside the extraction folder.
parent 66aa08f7
......@@ -180,6 +180,14 @@ void Job::onError(const QString & message, const QString & details)
void Job::onEntry(Archive::Entry *entry)
{
const QString entryFullPath = entry->fullPath();
if (QDir::cleanPath(entryFullPath).contains(QLatin1String("../"))) {
qCWarning(ARK) << "Possibly malicious archive. Detected entry that could lead to a directory traversal attack:" << entryFullPath;
onError(i18n("Could not load the archive because it contains ill-formed entries and might be a malicious archive."), QString());
onFinished(false);
return;
}
emit newEntry(entry);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment