GitLab

The CVE number for #12 has been assigned after the
release, so it is not part of the NEWS file. Let's add the CVE number
additionaly at least.

(Malicious) archives can have entries with symlink in parents. Archives
entries can have absolute paths, or relative paths that points outside
of the destination. Let's add test to ensure that extraction fails with
error for symlinks in parents and tests to verify that malformed paths
are correctly sanitized and not written outside.

(Malicious) archives can have malformed paths with `..` segments so they
point outside of the destination. The `autoar_extractor_do_sanitize_pathname`
function already sanitizes those paths to be inside of the destination,
however, the code from `autoar_extractor_step_decide_destination` operates on
paths, which are not yet sanitized and fails with the following criticals:
`g_file_resolve_relative_path: assertion 'relative_path != NULL' failed`.
Let's use the `autoar_extractor_do_sanitize_pathname` also here to fix this
criticals.

Currently, it is still possible that some files are extracted outside of
the destination dir in case of malicious archives. The checks from commit
adb067e6 can be still bypassed in certain cases. See file-roller#108
for more details. After some investigation, I am convinced that it would be
best to	simply disallow symlinks in parents. For example, `tar` fails to
extract such files with the `ENOTDIR` error. Let's do the same here.

Fixes: #12

This reverts commit adb067e6.

This reverts commit cc4e8b7c.

In case of conflict, when skipping some file, the `total_size` and `total_files`
is not updated, but neither `completed_files` and `completed_size`. Let's
reduce the `total_size` and `total_files`. Same approach is used in Nautilus
when skipping.

The symlink, or hardline should be rewriten itself, not its target.
Let's add tests to verify this.

Currently, symlinks are followed when detecting conflicts. But this
is not desired as the original file caused the conflict, not its target.

To be honest, it is not really clear to me what is purpose of this test.
As per the name, it should verify that error is returned when overwriting
file over directory. However, I think that it is totally fine to overwrite
empty directory. Anyway, the overwrite action is not explicitely chosen,
so the skip action is used instead. Consequently, the test verifies that
`error` is not set. So it looks to me that the test is tottaly wrong. Let's
modify and rename the test, so it really checks that error is returned when
somebody tries to overwrite non-empty directory.

Current logic doesn't detect conflics when extracting directory. This
is ok, but only for the case when the conflic is caused by directory.
Otherwise, the conflic should be detected and AutoarExtractor should
try to delete the file before creating new directory.

Currently, `g_file_replace` is used to write files. However, it uses
`G_FILE_CREATE_NONE` which keeps old permissions. It should rather use
`G_FILE_CREATE_REPLACE_DESTINATION` instead to not keep any old permissions
as it is among others used by File Roller. However, there is bug in
`G_FILE_CREATE_REPLACE_DESTINATION` implementaion, see glib#2325.
Let's explicitely delete that file and use `g_file_create` instead.
This will also fix problems when overwriting file by directory and
ensures that hardlinks will be replaced and not just modified.

From the code, it was not really clear what is the default action
for conflicts. Let's add test which verifies that conflicting files
are skipped by default.

The tests for conflicts contains several bugs and also are not able to
distingues between skip and overwrite actions. Let's modify the test so
they can really verify whether the files are skipped, or overwritten.

The test for conflict contains `test-one-file-` prefix which doesn't make
much sense here and just makes the name too long. Let's use just `test-`
prefix instead.

The `AUTOAR_CONFLICT_OVERWRITE` is set as default value for the action
variable when conflict occured. However, `g_signal_emit` clears that
variable to `0` if the signal is unhandled. But `0` is currently mapped
to `AUTOAR_CONFLICT_SKIP`. So the code is a little bit confusing. I think
that overwrite is the right thing in most cases and also this is the
default behavior of `tar` as an archive may contain several versions of
some file and the last one is the newest. However, gnome-autoar allows
extraction in the non-empty folders and has conflict API, so it would be
really safer to use the skip action by default. Let's add the
`AUTOAR_CONFLICT_UNHANDLED` action for better control and use the
`AUTOAR_CONFLICT_SKIP` action by default.

A test for the `output-is-dest` property is missing currently. Let's add
one to be sure that extra directory is not created for an archive with
a file with a different name.

If the `output-is-dest` property is `TRUE`, the `prefix` is not cleared and is
passed to `decide-destination` signal. This looks unexpected because it allows
to change even the prefix which doesn't match archive name, which is not allowed
even if `output-is-dest` is `FALSE`. I am conviced that it should not be allowed
to change the `prefix` at all in this case. Let's clear the `prefix` variable to
avoid that.

At the beginning, the file list is printed in the debug output. However,
it doesn't contain targets of symlinks and hardlinks. Let's print them
as well. Also print symlink target when writing it on the disk similary
to hardlinks.

`g_file_make_directory_with_parents` is called to create `self->destination_dir`
directory before extraction. However, the files may be written to completely
different dir later if the they have common prefix and the prefix is consequently
changed over `decide-destination` signal. Let's use `self->prefix_new` if it is
set to prevent creation of unrelated directories.

The returned value from `g_file_get_path` is not consequently freed.
Let's use `g_file_peek_path` instead to fix the leak.

The documentation refers to non-existing functions and properties.
It also contains misleading info about behavior of some functions,
or their parameters. Let's try to make the documentation clearer.
This also fixes some typos.

Recursive delete has been added by commit 58ac8fc5 to remove already created
directories when extraction fails because of an invalid password. In fact,
it deletes the whole `destination_dir` also in case of other failures, which
is maybe not the best approach, but ok. However, a problem is that
gnome-autoar allows extraction in non-empty destination, so this might remove
also files which were not initially created by gnome-autoar. Fortunately,
nautilus and gnome-shell currently always extracts in an extra directory. But
what is worse is the fact, that if the files in the archive have a common
prefix, then the `destination_dir` is actually a parent of that extra directory
in the case of nautilus and gnome-shell (but API allows to set completely
unrelated path)! So this can easily cause huge data loss! It would probably be
better to create parent directories only when `archive_read_data_block`
succeeds instead of deleting them later. Alternatively, gnome-autoar could
track which files were written and deletes just those on that list. But for now,
let's just remove the code for recursive delete and do not care about leftover
files...

The commit 66cf03f9 dropped support for RAR archives because they were not well
supported by libarchive. But a lot of changed since then and libarchive 3.4.0
came with RAR 5 support. Let's add back support for RAR and bump the libarchive
dependency accordingly.

Fixes: #2

Currently, it is not possible to extract archives that don't explicitly
contain parent folders. This is unintentional regression caused by commit
adb067e6. Let's simply ignore G_IO_ERROR_NOT_FOUND errors when looking
for symlinks to fix this.

Fixes: https://gitlab.gnome.org/GNOME/gnome-autoar/-/issues/11

It's auto-indented might be due to the comment above it.

The application/x-gzip is already covered by its alias
application/gzip.

Autoar can't detect archive with non-standard unsupported content type
that actually an alias of existing type supported.
To avoid listing all those alias, instead, we check
for similar type through g_content_type_equals.

Fixes #8

AutoarExtractor used to error out when libarchived reported an
archive entry to be encrypted.

We now emit a "passphrase-requested" signal that clients can
connect to return a single passphrase string which libarchive
will attempt to use to decrypt the files. For an empty or invalid
password, AutoarExtrator will emit its "error" signal and cleanup
the destinatination directory.

Although libarchive supports different password for each item,
this implementation assumes the same password for all entries
in a given archive. Single password per archive seems to be the
most common use-case for regular desktop end users.

See nautilus#327

Fixes #1

Rename the function parameter of autoar_extractor_set_delete_after_extraction
so it matches the name of the property it sets.

Following compile warnings showed up when link, mkfifo and mklink
weren't defined:

../gnome-autoar/autoar-extractor.c: In function ‘autoar_extractor_do_write_entry’:
../gnome-autoar/autoar-extractor.c:993:17: warning: unused variable ‘uname’ [-Wunused-variable]
  993 |     const char *uname;
      |                 ^~~~~
../gnome-autoar/autoar-extractor.c:1022:17: warning: unused variable ‘gname’ [-Wunused-variable]
 1022 |     const char *gname;
      |                 ^~~~~
../gnome-autoar/autoar-extractor.c:1237:1: warning: label ‘applyinfo’ defined but not used [-Wunused-label]
 1237 | applyinfo:
      | ^~~~~~~~~
../gnome-autoar/autoar-extractor.c:941:7: warning: variable ‘r’ set but not used [-Wunused-but-set-variable]
  941 |   int r;
      |       ^

Currently, a malicious archive can cause that the files are extracted
outside of the destination dir. This can happen if the archive contains
a file whose parent is a symbolic link, which points outside of the
destination dir. This is potentially a security threat similar to
CVE-2020-11736. Let's skip such problematic files when extracting.

Fixes: #7

This project is crucial for Nautilus, but it is obviously unmaintained
currently. I took over Nautilus maintainership from Carlos. So I hope
it doesn't mind to take over gnome-autoar as well.

See https://wiki.gnome.org/Projects/GnomeCommon/Migration