pkg_add.c: Resolve the shlib symlink before recursing into pkg_add
This subtle bug can be reproduced when pkg_add recurses through different
types of dependencies - deps and requires. The path passed into
pkg_add_check_pkg_archive() is expected to start with "All/", but after
handling a shlib requirement and recursing down it changes to "shlibs/".
At the bottom the code ends up doing
access("/../shlibs/libconsumer.so.1/package1-1.pkg",F_OK) ERR#2 'No such file or directory'
instead of
access("/../All/libconsumer.so.1/package1-1.pkg",F_OK) 0 = (0x0)
The added test case fails without this fix.
checksum: report the path of a file whose digest fails
The digest helpers reported the failure with their own name and without
the path, as in "pkg_checksum_hash_blake2_file(read failed): Is a
directory". They are silent now, pkg_checksum_fileat() reports
"Unable to open <path>" or "Unable to compute the checksum of <path>"
followed by the system error.
checksum: do not free an unallocated pointer on a read error
The blake2 and blake2s file digests freed *out before setting it, so an
unreadable path, a directory listed as a file for instance, crashed
pkg create. All four now clear the output and its length on failure, and
pkg_checksum_fd() initializes them.
utils: compare the canonical target with the root directory
pkg_resolve_beneath() re-roots an absolute symlink target when it does
not name a path inside of the root, but the root directory is canonical
while the target may be spelled through a symlinked prefix: the two do
not compare equal.
Every such target was then re-rooted, which made the resolution walk the
same symlink again until the hop limit, and pkg failed with "Unable to
resolve ... inside ...". Compare the canonical form of the target as
well, so that a path which is inside of the root is recognised whatever
the spelling is, and the chain settles.
build: do not abort when xlocale.h is missing
configure runs with set -e and cc_check_includes returns non zero when a
header is not found, so the probe added for the libfetch locale support
stopped the configuration on the systems which do not ship xlocale.h,
Debian and Alpine among them.
The header only selects the fast path: the bundled libfetch falls back
to setlocale() when any of the four probes fails.
osvf: version ranges collapsed every events list into a single interval
pkg_osvf_parse_events() wrote each "introduced" into v1 and each
"fixed" into v2 of the one range allocated per ranges[] entry, so an
events list describing several intervals, such as
[introduced 1.0, fixed 1.5, introduced 2.0, fixed 2.3], ended up as
the single interval [2.0, 2.3] and leaked the earlier strings. It also
mapped "fixed" to an inclusive bound although OSV defines it as the
first version that is no longer affected, and ignored "last_affected"
and "limit" entirely, leaving such ranges open ended.
Once an OSV feed is wired into pkg audit this would have missed every
interval but the last and reported the fixed version as vulnerable.
The parser now walks the events in order: "introduced" opens a new
range, "fixed" and "limit" close it exclusively and "last_affected"
inclusively, each interval becoming its own range. A ranges[] entry
without events still means every version is affected. The test fixture
gains a package with two intervals and the expectations are adjusted.
pkgdb: SQL injection through the manifest dep_formula field
pkg_deps_formula_tosql() spliced the dependency names and versions of a
package's dep_formula into an SQL WHERE clause with snprintf() and no
escaping, and pkgdb_load_deps() ran the result against local.sqlite.
The formula parser accepts any printable character except space and
comma in a name, including the single quote.
dep_formula is an ordinary manifest string stored at registration and
evaluated every time the package is loaded with its dependencies, so a
malicious package could inject arbitrary SQL into that SELECT, for
example "x')OR('1'='1" to make every installed package a dependency of
itself and distort the solver, autoremove and pkg delete for unrelated
packages. Only a single statement is compiled and no extension loading
is enabled, so the injection is limited to reads.
The clause is now built with the string builder and every value is
appended with its quotes doubled, so it cannot leave the SQL literal.
tests: cover a plist line holding only blanks
Such a line reaches the parser as an empty path; the trailing space
trim used to walk backwards past the start of the heap buffer.
pkgdb: reverse dependency rows were never updated on upgrade
When a package is registered, pkgdb refreshes the origin and version
recorded in the deps rows of the packages that depend on it. Since the
switch to typed SQL arguments, the call passes the version twice, so
the name placeholder was bound to the version string and the actual
name went to a non-existent fourth parameter.
The UPDATE therefore matched no rows and every dependent kept the
origin and version of the dependency as it was when the dependent was
installed. pkg info -d and %do showed stale origins after a port moved,
and pkg query -e conditions on %dv and %do evaluated against outdated
data.
Pass the arguments in the order the statement expects.
repo: an unparsable catalogue replaced the repository with an empty one
When the JSON catalogue in the data archive could not be parsed, or was
not an object, the update code reported "Invalid data" and jumped to the
cleanup path with rc still holding the success of the preceding
transaction start.
The cleanup then committed the empty new database, discarded the saved
copy of the previous one and returned success, so pkg update announced
the repository as updated. A truncated or corrupt catalogue from a
mirror left the repository with no packages, and because the database
mtime was stamped as current, later updates said it was up to date
until a forced update.
Fail the update in those cases so the previous catalogue is restored.
repo: catalogue paths could escape the cache directory
The repository catalogue's "path" (repopath), "name" and "version"
fields were used verbatim to build the destination file name under
the cache directory, and under the mirror directory for pkg fetch -o,
with no check for ".." components or embedded slashes.
pkg_mkdirs() creates every intermediate directory and treats an
existing one as success, so "All/x.pkg/../../../etc/rc.local" was
created-then-traversed exactly as written, and the extension taken
from the last '.' of such a path carried the traversal into the
normal cache name as well.
An unsigned repository (the default signature_type), an http MITM or
a malicious mirror could therefore make pkg fetch, running as root,
append attacker bytes to any file, delete any existing file by
listing it with a wrong size, and then recreate it with chosen
content whose checksum matched the catalogue. No install was needed,
and pkg fetch -o DIR escaped the chosen directory.
[4 lines not shown]
pkg_abi_from_string: minor OS version parsed only when the major failed
For OSes whose ABI carries a major.minor version (DragonFly, Linux)
the minor was parsed inside "if (errstr != NULL)", i.e. only after the
major had already failed to parse, and a successful minor parse then
cleared errstr. A valid "dragonfly:6.4:x86:64" therefore always ended
up with minor 0, which feeds the VERSION_MINOR config variable and the
solver's ABI ordering, while an invalid "dragonfly:abc.4:x86:64" was
accepted with major 0.
Parse the minor when the major succeeded and reject the string when
either part is invalid; the wildcard check that follows is left to the
major-only OSes since strsep() has consumed the version by then.
scripts: a script exiting with status 3 made pkg exit(0) mid-transaction
pkg_script_run_child() called exit(0) from inside libpkg whenever a
package script, in any phase, exited with status 3. This dates from
983eb7236, which wanted the kernel package's post-install script to be
able to say "reboot before upgrading the rest" and was unsure how to
do it; it was never documented.
The effect was far wider than intended. A pre-deinstall script with
"exit 3" turned pkg delete into a successful no-op: the process ended
with status 0 before the package was unregistered. A post-deinstall
"exit 3" left the files removed but the package registered. A
post-install "exit 3" ended pkg upgrade right there with status 0, so
every package scheduled after it was silently not upgraded while cron
or configuration management saw success, and the deferred rc restarts,
transaction triggers and explicit lock release never ran. During an
upgrade the old package's pre-deinstall ran with the new package
registered inside an open transaction, so exit(0) rolled the
registration back and left .pkgtemp files on disk.
[9 lines not shown]
tests: the rootdir symlink escape is fixed
An intermediate symlink with an absolute target no longer redirects
an install onto the host. An absolute symlink pointing inside the
rootdir is now refused rather than followed, which remains a deviation
from the previous behaviour and is still an expected failure.
libfetch: use a C locale for the protocol dates
The dates exchanged with a repository, the Last-Modified of a response
and the If-Modified-Since of a request, are always written in the C
locale. http_parse_mtime() saved the locale of the process, replaced
it with the C one and restored it, which changes global state every
time a date is parsed and is not safe if the library is used from
several threads; the request side formatted the date in whatever the
process locale was.
Keep one C locale and use strptime_l() and strftime_l() with it, as
done in FreeBSD.
The functions are not available everywhere: configure checks for
xlocale.h, newlocale(), strptime_l() and strftime_l(), and the previous
behaviour is kept when they are missing.
repo: group names from the repository were used as printf formats
pkg_repo_binary_group_it_next() built the group's name and comment
with xasprintf() passing the repository-supplied string as the format
argument. A group named "%n%n%s" in the repository's groups file is
therefore interpreted by vasprintf(), giving a repository operator, or
anyone who can write the repository directory, a write-what-where in
every "pkg search" and "pkg install @group" run as root. A missing
name also passed a NULL format.
The strings are now copied with xstrdup(), entries without a string
name are skipped, and a missing comment becomes an empty string.
plist: heap underflow on a whitespace-only line
meta_file() trimmed trailing whitespace with a loop that read
line[len - 1] without first checking that len is non-zero. A plist
line consisting only of blanks (or a bare "@ ") reaches it as an
empty string after plist_parse_line() has skipped the leading blanks,
so the loop read and wrote NUL bytes backwards through the heap
buffer for as long as the preceding bytes happened to be whitespace.
A crafted pkg-plist handed to pkg create or pkg register could thus
corrupt the heap of the process building the package.
The trim loop now stops at the start of the string and an empty path
is rejected with an error.
tests: cover symlink handling inside a rootdir
A package may legitimately be installed through a directory symlink
already present in the target, so check the relative, absolute and
dangling cases. The last case must not write outside of the rootdir;
it is currently an expected failure.
docs: Document how the lua script paths are rooted
pkg-lua-script(5): only io.open(), os.remove(), os.rename(), pkg.stat()
and pkg.readdir() resolve their paths relatively to the root directory
passed with -r, pkg.exec() and pkg.exec_capture() spawn the command on the
host and are therefore given host paths.
Document that pkg.stat() does not follow symbolic links, returns nil when
the file does not exist and the values the type field can take, and that
pkg.readdir() returns nil and an error message when the directory cannot
be opened.
pkg-keywords(5): document the %@ substitution next to %<number> and warn
that, the substitution being textual, a literal %1 in a lua string (e.g.
the replacement string of string.gsub()) is substituted as well.
triggers: per-package triggers from a -r target ran on the host
When pkg operates on another root (-r), non-sandboxed transaction
triggers found inside that root are not executed but written to the
target's db directory, to be run later by "pkg triggers" from inside
the target: code from the target must not run on the host. Per-package
triggers bypassed that: triggers_execute_perpackage() called the Lua
runner with deferral disabled, so a "sandbox: false" trigger shipped
under the target's PKG_TRIGGERS_DIR was forked and executed on the
host as root, with the host's PATH and environment and pkg.exec
available.
A build host populating a jail or image with pkg -r would thus run
arbitrary code from any package present in the image on the next -r
operation touching a matching path.
Per-package triggers are now deferred like transaction triggers. The
deferred file gains "--pkgname", "--pkgversion" and "--upgrade" header
lines so that pkg triggers can hand the package context back to the
[4 lines not shown]
pkg_add: a symlink inside a -r root could redirect installs onto the host
All file operations during extraction and removal are *at() calls
relative to the root directory fd with the leading slash stripped, and
only the last path component is protected with O_EXCL or
AT_SYMLINK_NOFOLLOW. The kernel resolves every parent component, and
when one of them is a symlink with an absolute target, or a relative
one climbing above the root with "..", resolution restarts at the
process root rather than at the root fd. open_tempdir() even follows
symlinks on purpose when stat'ing the parents of a fresh install.
When pkg populates another root with -r (jails, images, build hosts)
a package shipping /usr/local/etc/periodic -> /etc/periodic followed
by a dependent package installing /usr/local/etc/periodic/daily/999.x
therefore wrote that file into the host's /etc/periodic/daily, and
pkg -r delete unlinked host files the same way. No script is needed,
so -I does not help, and Lua scripts, supposed to be confined to the
root fd, inherit the same gap.
[6 lines not shown]
pkg_add: directory entries changed the attributes of pre-existing directories
create_dir() tolerated an existing directory (mkdirat EEXIST is
success) without remembering that it had not created it, and only set
noattrs when the directory already carried exactly the requested
owner, group and mode. pkg_extract_finalize() then applied the
archive's or manifest's owner, group, mode and file flags to every
directory the package listed, and the deletion path never restores
anything.
A package merely listing /etc with mode 0777, /tmp without the sticky
bit, or /usr/local/etc with the schg flag, therefore silently made the
system directory world-writable (local root for any user), let any
user delete other users' temporary files, or locked every other
package's configuration, with nothing printed and the change outliving
the package. Directories listed in the manifest but absent from the
archive got uid 0, gid 0 and whatever mode the manifest said, with the
same effect.
[8 lines not shown]
pkg register: post-install scripts ran after a failed installation
pkg_add_port, used by pkg register to install from a staging
directory, ran the post-install Lua and shell scripts and the
per-package post-install triggers whether or not extracting the files
had succeeded, after the failed installation had already been rolled
back.
A port whose staging area was incomplete failed to register, left no
package installed, and still had its post-install script run against
files that were not there, for example adding users, enabling services
or editing configuration for a package that does not exist.
Only run the post-install phase when the installation succeeded.
repo: meta file that failed verification was kept, via an uninitialised path
pkg_repo_fetch_meta() declared a filepath buffer that was never
assigned and passed it to unlinkat() on both error paths. In the legacy
meta.txz flow the meta is extracted into the repository's real "meta"
file before the fingerprint and signature checks run, so when those
failed the cleanup unlinked stack garbage instead and the unverified
meta stayed on disk, where the repository open code loads it on every
later run while pkg update only downgraded the failure to a notice.
Unlink the "meta" entry of the repository directory itself on failure.
tests: cover a script killed by a signal
A child killed by a signal has an exit status of 0 once shifted, so
its death used to be reported as a success. Check that a pre-install
script killed by SIGSEGV fails the install and is reported as such.
pkg_add: temporary files and directories were left behind by a failed extraction
pkg_rollback_pkg() passed the absolute temporary path to unlinkat()
with the root fd, so the fd was ignored and the host path unlinked
instead; under -r that path does not exist and the half extracted
.pkgtemp file stayed inside the target. The temporary directories
created by open_tempdir(), and the files already extracted into them
(which carry no temppath of their own), were never removed at all on
failure, under any root.
Use the root relative path for the rollback and remove every temporary
directory tree when the extraction fails. The recursive removal is
factored out of the cache cleaner into pkg_rmtree_at() so both share
one implementation.
