From 0a394144b22f02b5956e89833ff91e9e232a3e17 Mon Sep 17 00:00:00 2001 From: Andrew Gregory Date: Sun, 4 Feb 2024 11:47:18 -0800 Subject: [PATCH] validate package metadata after loading alpm has certain requirements for package metadata necessary for proper functioning, name and version in particular. These requirements are already enforced in makepkg, but nowhere in alpm. Exceptions are treated as errors for non-local packages because they cannot be installed without potentially resulting in undefined behavior. Exceptions for local packages are treated as warnings because they are already installed, so any damage has already been done, and the user would otherwise have no way to uninstall them. Signed-off-by: Andrew Gregory --- lib/libalpm/be_local.c | 4 ++ lib/libalpm/be_package.c | 4 ++ lib/libalpm/be_sync.c | 5 ++ lib/libalpm/package.c | 56 +++++++++++++++++++ lib/libalpm/package.h | 2 + test/pacman/meson.build | 3 + .../tests/pkg-meta-invalid-name-file.py | 9 +++ .../tests/pkg-meta-invalid-name-local.py | 9 +++ .../tests/pkg-meta-invalid-name-sync.py | 9 +++ 9 files changed, 101 insertions(+) create mode 100644 test/pacman/tests/pkg-meta-invalid-name-file.py create mode 100644 test/pacman/tests/pkg-meta-invalid-name-local.py create mode 100644 test/pacman/tests/pkg-meta-invalid-name-sync.py diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index a0e7108b..349e84b7 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -630,6 +630,10 @@ static int local_db_populate(alpm_db_t *db) continue; } + /* treat local metadata errors as warning-only, + * they are already installed and otherwise they can't be operated on */ + _alpm_pkg_check_meta(pkg); + /* add to the collection */ _alpm_log(db->handle, ALPM_LOG_FUNCTION, "adding '%s' to package cache for db '%s'\n", pkg->name, db->treename); diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index f8e51f3c..cddbc01e 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -673,6 +673,10 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, newpkg->infolevel |= INFRQ_FILES; } + if(_alpm_pkg_check_meta(newpkg) != 0) { + goto pkg_invalid; + } + _alpm_archive_read_free(archive); close(fd); return newpkg; diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 588895e5..8d6795bf 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -344,6 +344,11 @@ static alpm_pkg_t *load_pkg_for_entry(alpm_db_t *db, const char *entryname, pkg->ops = get_sync_pkg_ops(); pkg->handle = db->handle; + if(_alpm_pkg_check_meta(pkg) != 0) { + _alpm_pkg_free(pkg); + RET_ERR(db->handle, ALPM_ERR_PKG_INVALID, NULL); + } + /* add to the collection */ _alpm_log(db->handle, ALPM_LOG_FUNCTION, "adding '%s' to package cache for db '%s'\n", pkg->name, db->treename); diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 8c95dab3..50bbe591 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -21,6 +21,7 @@ * along with this program. If not, see . */ +#include #include #include #include @@ -844,3 +845,58 @@ int SYMEXPORT alpm_pkg_should_ignore(alpm_handle_t *handle, alpm_pkg_t *pkg) return 0; } + +/* check that package metadata meets our requirements */ +int _alpm_pkg_check_meta(alpm_pkg_t *pkg) +{ + char *c; + int error_found = 0; + +#define EPKGMETA(error) do { \ + error_found = -1; \ + _alpm_log(pkg->handle, ALPM_LOG_ERROR, error, pkg->name, pkg->version); \ +} while(0) + + /* sanity check */ + if(pkg->handle == NULL) { + return -1; + } + + /* immediate bail if package doesn't have name or version */ + if(pkg->name == NULL || pkg->name[0] == '\0' + || pkg->version == NULL || pkg->version[0] == '\0') { + _alpm_log(pkg->handle, ALPM_LOG_ERROR, + _("invalid package metadata (name or version missing)")); + return -1; + } + + if(pkg->name[0] == '-' || pkg->name[0] == '.') { + EPKGMETA(_("invalid metadata for package %s-%s " + "(package name cannot start with '.' or '-')\n")); + } + if(_alpm_fnmatch(pkg->name, "[![:alnum:]+_.@-]") == 0) { + EPKGMETA(_("invalid metadata for package %s-%s " + "(package name contains invalid characters)\n")); + } + + /* multiple '-' in pkgver can cause local db entries for different packages + * to overlap (e.g. foo-1=2-3 and foo=1-2-3 both give foo-1-2-3) */ + if((c = strchr(pkg->version, '-')) && (strchr(c + 1, '-'))) { + EPKGMETA(_("invalid metadata for package %s-%s " + "(package version contains invalid characters)\n")); + } + if(strchr(pkg->version, '/')) { + EPKGMETA(_("invalid metadata for package %s-%s " + "(package version contains invalid characters)\n")); + } + + /* local db entry is - */ + if(strlen(pkg->name) + strlen(pkg->version) + 1 > NAME_MAX) { + EPKGMETA(_("invalid metadata for package %s-%s " + "(package name and version too long)\n")); + } + +#undef EPKGMETA + + return error_found; +} diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index 5ebe6bd1..c2313d35 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -165,4 +165,6 @@ int _alpm_pkg_compare_versions(alpm_pkg_t *local_pkg, alpm_pkg_t *pkg); alpm_pkg_xdata_t *_alpm_pkg_parse_xdata(const char *string); void _alpm_pkg_xdata_free(alpm_pkg_xdata_t *pd); +int _alpm_pkg_check_meta(alpm_pkg_t *pkg); + #endif /* ALPM_PACKAGE_H */ diff --git a/test/pacman/meson.build b/test/pacman/meson.build index 655005a4..c26ce0fa 100644 --- a/test/pacman/meson.build +++ b/test/pacman/meson.build @@ -97,6 +97,9 @@ pacman_tests = [ 'tests/pacman003.py', 'tests/pacman004.py', 'tests/pacman005.py', + 'tests/pkg-meta-invalid-name-file.py', + 'tests/pkg-meta-invalid-name-local.py', + 'tests/pkg-meta-invalid-name-sync.py', 'tests/provision001.py', 'tests/provision002.py', 'tests/provision003.py', diff --git a/test/pacman/tests/pkg-meta-invalid-name-file.py b/test/pacman/tests/pkg-meta-invalid-name-file.py new file mode 100644 index 00000000..49060340 --- /dev/null +++ b/test/pacman/tests/pkg-meta-invalid-name-file.py @@ -0,0 +1,9 @@ +self.description = "package name with invalid characters cannot be installed (file)" + +p = pmpkg("-foo") +self.addpkg(p) + +self.args = "-U -- %s" % p.filename() + +self.addrule("!PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=-foo") diff --git a/test/pacman/tests/pkg-meta-invalid-name-local.py b/test/pacman/tests/pkg-meta-invalid-name-local.py new file mode 100644 index 00000000..2789abdd --- /dev/null +++ b/test/pacman/tests/pkg-meta-invalid-name-local.py @@ -0,0 +1,9 @@ +self.description = "local package name with invalid characters can be removed" + +sp = pmpkg("-foo") +self.addpkg2db("local", sp) + +self.args = "-R -- %s" % sp.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=-foo") diff --git a/test/pacman/tests/pkg-meta-invalid-name-sync.py b/test/pacman/tests/pkg-meta-invalid-name-sync.py new file mode 100644 index 00000000..895facbf --- /dev/null +++ b/test/pacman/tests/pkg-meta-invalid-name-sync.py @@ -0,0 +1,9 @@ +self.description = "package name with invalid characters cannot be installed" + +sp = pmpkg("-foo") +self.addpkg2db("sync", sp) + +self.args = "-S -- %s" % sp.name + +self.addrule("!PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=-foo")