GitLab

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.

Summary: Same observation and resolution as in D25565

Test Plan: Verified performance gain with hotspot

Reviewers: #ark, rthomsen

Reviewed By: #ark, rthomsen

Subscribers: rthomsen, dakon, kde-utils-devel

Tags: #ark

Differential Revision: https://phabricator.kde.org/D26356

Errors from ExtractJob are not handled when called as a subjob of
BatchExtractJob. This can result in silently failing extractions.

CCBUG: 387996
Differential Revision: D28721

Test Plan: builds

Reviewers: #ark, elvisangelaccio

Reviewed By: #ark, elvisangelaccio

Subscribers: kde-utils-devel

Tags: #ark

Differential Revision: https://phabricator.kde.org/D25706

Summary: Treat single files archive as single folder archive basically.

Test Plan: ctest

Reviewers: #ark, elvisangelaccio, ngraham

Reviewed By: #ark, elvisangelaccio

Subscribers: kde-utils-devel

Tags: #ark

Differential Revision: https://phabricator.kde.org/D23693

Summary:
Another attempt at fixing #402824 after ce2f7f66.

This should fix progress reporting also for libzip and libarchive plugins.
The problem with those plugin is that CreateJob starts two nested threads.

BUG: 402824
FIXED-IN: 19.04.3

Reviewers: rthomsen

Subscribers: kde-utils-devel, #ark

Tags: #ark

Differential Revision: https://phabricator.kde.org/D21686

Evolution of b1a251eb. We no longer want to distinguish between
libarchive and libzip. If there are crashes in libarchive because of
this new kill logic, we need to fix them.

Summary: Port to for(...:...)

Reviewers: elvisangelaccio

Reviewed By: elvisangelaccio

Subscribers: kde-utils-devel, #ark

Tags: #ark

Differential Revision: https://phabricator.kde.org/D21515

Progress info is currently broken when creating archives using
CreateJob. This happens e.g. when the user creates an archive
through the KFileItemAction in Dolphin.
This is caused by CreateJob using a nested AddJob and the percent
signal of the AddJob was not forwarded to CreateJob.

It was encessary to use QOverload since KJob has both a signal and a
member function called percent.

BUG: 382599
FIXED-IN: 19.04.0
Differential Revision: D19953

These days Qt offers the QT_MESSAGE_PATTERN env variable, so function
names can be automatically added to each debug output line using the
`%{function}` placeholder.

It doesn't make sense to set these properties if loading the archive
failed. This also allows to fix the expected values of a testcase.

The libzip plugin sometimes cannot react to `QThread::requestInterruption()`
because there is a blocking `zip_close()` which writes to disk (e.g. with AddJobs).

This means we have to manually abort the thread (by passing a timeout to
`QThread::wait()` in `Job::doKill()`. We cannot do this uconditionally
because we would end up with crashes in libarchive. Since the libarchive
plugin is not affected by this problem, we rework the logic in
`Job::doKill()` by assuming that the interface will tell us whether it
needs to be brutally killed. This way we can distinguish between the
libarchive and the libzip plugins (the ones that use a worker thread).

The mutex in this patch is needed because in theory `m_operationMode` could be
read and written by different threads at the same time, even though that's unlikely.

BUG: 389290
FIXED-IN: 18.03.80
Task: T7824

Whether the job emits the `result()` signal should depend on the
argument passed to `KJob::killed()`. From the Ark side we always call
`kill()` which uses the default argument `Quietly`.

However, KUiServerJobTracker (called from the plasma systrat applet)
calls `kill(KJob::EmitResult)`. This means that the result signal will
be emitted twice: one from `KJob::finishJob()` and another one when the
Ark jobs return and we go to `Job::onFinished()`.

This means that from the Ark side we need to emit `result()` only when
we didn't kill the job quietly, i.e. when we didn't call `doKill()`
(which sends the request interruption to the secondary threads).

This patch does that but it's quite an hack. A better long-term solution
would be to refactor the interface functions so that they return an enum
rather than a boolean (see T8084).

Task: T8081

If the user canceled an OverwriteQuery dialog, we should not claim the
extraction finished. By emitting the canceled() signal, the extraction
jobs will set the KilledJobError error and the plasma applet will not
show the "Finished" string in the notification.

BUG: 382601
FIXED-IN: 17.12.3

Task: T6707

`CreateJob::doWork()` might be executed in another thread (if we are
using the libarchive or libzip plugin), so any QObject created there
cannot use `this` as parent (since the CreateJob had been created
in the main thread).

We don't actually need to create an Entry object here, since all the
interfaces have checks for null destination entries.

Fixes T7206

The plasma notification applet looks for it for the "Open" button within
the notification. If it doesn't find it, it falls back to the archive
URL which is not what one would expect.

BUG: 385043
FIXED-IN: 17.11.90

Differential Revision: D8861

It was added in commit e1bc57bc but it's never been used.

Summary:
There are two entries with "Extracting one file". The one is called with
i18n and the other is called with i18np. Thus it confuses the translation
to use "Extracting %1 files" in i18n() version which is a problem for
languages with "Plural-Forms: nplurals=1; plural=0;". The actual result
before this fix will be using the "%1" without arguments.

Test Plan: Manually tested.

Reviewers: rthomsen, elvisangelaccio

Reviewed By: elvisangelaccio

Subscribers: kde-utils-devel, #ark

Tags: #ark

Differential Revision: https://phabricator.kde.org/D5511

The `virtual` keyword is useless when `override` is also used.

Patch generated by:

run-clang-tidy.py -header-filter='.*' -checks='-*,modernize-use-override' -fix

Patch generated by:

git grep -l 'Q_DECL_OVERRIDE' | xargs sed -i 's/Q_DECL_OVERRIDE/override/g'

Path generated by:

git grep -l 'Q_NULLPTR' | xargs sed -i 's/Q_NULLPTR/nullptr/g'

Patch generated by:

cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
run-clang-tidy.py -header-filter='.*' -checks='-*,modernize-use-nullptr' -fix

'Adding a file' is too generic, 'Compressing a file' is what is actually
going on.

CCBUG: 377471

Differential Revision: D5027

Both AddToArchive and CreateJob are wrapper jobs, they need to implement
doKill() and call kill() on the actual jobs that are doing the work.

BUG: 374433
FIXED-IN: 16.12.1

Differential Revision: D4000

Instead of using uint or int in differenct places.

GIT_SILENT

We need to reimplement `KJob::doKill()` both in `BatchExtract` and
`BatchExtractJob`. In the latter class we use an enum to keep track of which
sub-job we are running and kill it when necessary.

Differential Revision: https://phabricator.kde.org/D3521

BatchExtractJob runs a LoadJob first and an ExtractJob after, which results in two
different progress bars in the notification tray. This patch changes how the
percentage is computed: the first 50% is from the LoadJob, the 2nd 50% from the
ExtractJob. This ensures that only one progress bar shows up while the wrapper job
is running. This will only work if the interface is able to report progress for
both LoadJobs and ExtractJobs (currently only libarchive and clirar).

Differential Revision: D3518

The new BatchExtractJob needs to forward the progress() signal (emitted
when the underlying ExtractJob runs) to its own onProgress() slot. This
works because the archiveInterface() instance is the same for both jobs.

The AddJob's description is now visible again in the system tray when
creating archives from Dolphin.

Differential Revision: D3371

The Query was still assuming that all plugins run from a secondary
thread.
Since CliInterface-based plugins run from the main thread, there is no need to
emit the `userQuery` signal and then call `Query::waitForResponse()`.
They can directly call `Query::execute()` instead.

Differential Revision: D3217

Currently `extracttest` has random failures when we check properties
with the libarchive plugin.

This happens because there is a race condition between
`LoadJob::onFinished()` (where we read some `LoadJob` members) and
`LoadJob::onNewEntry()` (where we write those members).

`onFinished()` is called when `list()` returns, in `LoadJob::doWork()`.
But that might happen before `onNewEntry()` is called, since they are executed
in two different threads.

This patch puts the `onFinished()` call in the event queue, just like the
single-thread case does (where we emit the `finished` signal from `CliInterface`).

Differential Revision: D3111

Jobs are not started when created.

Read-only interfaces cannot remove entries, so they should never emit this signal.

Differential Revision: D3090

Progress is now also shown in percentage for Addjob, CopyJob, DeleteJob
and MoveJob for archives handled by LibarchivePlugin. This was a bit
tricky due to libarchive always iterating the whole archive and means
that the plugin needs to know the total number of existing archive
entries.

A new member variable m_numberOfEntries was added to
ReadOnlyArchiveInterface, which holds the total number of entries in the
archive. The variable is kept up-to-date by incrementing/decrementing it
whenever the entry and entryRemoved signals are emitted by a plugin.
This necessitated a slight rework of the handling of MoveJob and CopyJob
by LibarchivePlugin because these emitted entry when iterating over the
old entries. The new approach should also be more efficient.

The two Archive members m_numberOfFiles and m_numberOfFolders were
removed.

ReadOnlyArchiveInterface::addFiles() got an additional argument of type
uint that holds the number of entries to be added.

Differential Revision: D3072

GIT_SILENT

Currently CompressionOptions and ExtractionOprions are both QHash typedefs,
which means they are the same thing for the compiler.
Currently we even pass CompressionOptions objects where ExtractionOptions objects are expected.
Both types are changed into proper classes, so that the compiler can
detect this class of bugs.

While at it:

- The default value for the `PreservePaths` option was false, now is true because it's
  the more common case.
- The `RemoveRootNode` option was redundant, it was only used together with the
  `DragAndDrop` one.
- The `FollowExtractionDialogSettings` was only set but never read, so we can drop it.

Differential Revision: D3039
Task: T2137

We never prepend to QList, so there is no reason for not using the more
efficient QVector.

Show archive name in notifications in Plasma system tray for various job
types.

Differential Revision: D2971