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]
