Radish alpha
H
HardenedBSD-pkg
rad:z3QDZAW2FAfuLvihrhiyDC9fAD8G9
HardenedBSD Package Manager
Radicle
Git
Commits › e482b66
extraction: fix an upgrade issue with symlinks becoming directories
Baptiste Daroussin committed 2 years ago
commit e482b6668b813359ad3658ad5c5c1432964e7b1b
parent e1e788d
6 files changed +78 -37
modified libpkg/pkg_add.c
@@ -258,7 +258,7 @@ set_attrsat(int fd, const char *path, mode_t perm, uid_t uid, gid_t gid,
	/* zfs drops the setuid on fchownat */

	if (fchmodat(fd, RELATIVE_PATH(path), perm, AT_SYMLINK_NOFOLLOW) == -1) {

		if (errno == ENOTSUP) {
- 
			/*
+ 
			/*

			 * Executing fchmodat on a symbolic link results in

			 * ENOENT (file not found) on platforms that do not

			 * support AT_SYMLINK_NOFOLLOW. The file mode of
@@ -319,7 +319,7 @@ reopen_tempdir(int rootfd, struct tempdir *t)
}



static struct tempdir *
- 
get_tempdir(int rootfd, const char *path, tempdirs_t *tempdirs)
+ 
get_tempdir(int rootfd, const char *path, tempdirs_t *tempdirs, stringlist_t *symlinks_allowed)

{

	struct tempdir *tmpdir = NULL;
@@ -330,7 +330,7 @@ get_tempdir(int rootfd, const char *path, tempdirs_t *tempdirs)
		}

	}
- 
	tmpdir = open_tempdir(rootfd, path);
+ 
	tmpdir = open_tempdir(rootfd, path, symlinks_allowed);

	if (tmpdir != NULL)

		tll_push_back(*tempdirs, tmpdir);
@@ -348,14 +348,14 @@ close_tempdir(struct tempdir *t)
}



static int
- 
create_dir(struct pkg *pkg, struct pkg_dir *d, tempdirs_t *tempdirs)
+ 
create_dir(struct pkg *pkg, struct pkg_dir *d, tempdirs_t *tempdirs, stringlist_t *symlinks_allowed)

{

	struct stat st;

	struct tempdir *tmpdir = NULL;

	int fd;

	const char *path;
- 
	tmpdir = get_tempdir(pkg->rootfd, d->path, tempdirs);
+ 
	tmpdir = get_tempdir(pkg->rootfd, d->path, tempdirs, symlinks_allowed);

	if (tmpdir == NULL) {

		fd = pkg->rootfd;

		path = d->path;
@@ -398,7 +398,7 @@ create_dir(struct pkg *pkg, struct pkg_dir *d, tempdirs_t *tempdirs)
/* In case of directories create the dir and extract the creds */

static int

do_extract_dir(struct pkg* pkg, struct archive *a __unused, struct archive_entry *ae,
- 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs)
+ 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs, stringlist_t *symlinks_allowed)

{

	struct pkg_dir *d;

	const struct stat *aest;
@@ -417,7 +417,7 @@ do_extract_dir(struct pkg* pkg, struct archive *a __unused, struct archive_entry
	fill_timespec_buf(aest, d->time);

	archive_entry_fflags(ae, &d->fflags, &clear);
- 
	if (create_dir(pkg, d, tempdirs) == EPKG_FATAL) {
+ 
	if (create_dir(pkg, d, tempdirs, symlinks_allowed) == EPKG_FATAL) {

		return (EPKG_FATAL);

	}
@@ -443,14 +443,15 @@ try_mkdir(int fd, const char *path)
}



static int
- 
create_symlinks(struct pkg *pkg, struct pkg_file *f, const char *target, tempdirs_t *tempdirs)
+ 
create_symlinks(struct pkg *pkg, struct pkg_file *f, const char *target, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	struct tempdir *tmpdir = NULL;

	int fd;

	const char *path;

	bool tried_mkdir = false;
- 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs);
+ 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs, symlinks_allowed);

	if (tmpdir == NULL && errno == 0)

		hidden_tempfile(f->temppath, sizeof(f->temppath), f->path);

	if (tmpdir == NULL) {
@@ -489,7 +490,8 @@ retry:
/* In case of a symlink create it directly with a random name */

static int

do_extract_symlink(struct pkg *pkg, struct archive *a __unused, struct archive_entry *ae,
- 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs)
+ 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	struct pkg_file *f;

	const struct stat *aest;
@@ -509,7 +511,7 @@ do_extract_symlink(struct pkg *pkg, struct archive *a __unused, struct archive_e
	fill_timespec_buf(aest, f->time);

	archive_entry_fflags(ae, &f->fflags, &clear);
- 
	if (create_symlinks(pkg, f, archive_entry_symlink(ae), tempdirs) == EPKG_FATAL)
+ 
	if (create_symlinks(pkg, f, archive_entry_symlink(ae), tempdirs, symlinks_allowed) == EPKG_FATAL)

		return (EPKG_FATAL);



	metalog_add(PKG_METALOG_LINK, RELATIVE_PATH(path),
@@ -520,7 +522,8 @@ do_extract_symlink(struct pkg *pkg, struct archive *a __unused, struct archive_e
}



static int
- 
create_hardlink(struct pkg *pkg, struct pkg_file *f, const char *path, tempdirs_t *tempdirs)
+ 
create_hardlink(struct pkg *pkg, struct pkg_file *f, const char *path, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	bool tried_mkdir = false;

	struct pkg_file *fh;
@@ -529,7 +532,7 @@ create_hardlink(struct pkg *pkg, struct pkg_file *f, const char *path, tempdirs_
	struct tempdir *tmpdir = NULL;

	struct tempdir *tmphdir = NULL;
- 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs);
+ 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs, symlinks_allowed);

	if (tmpdir == NULL && errno == 0)

		hidden_tempfile(f->temppath, sizeof(f->temppath), f->path);

	if (tmpdir != NULL) {
@@ -595,7 +598,8 @@ retry:


static int

do_extract_hardlink(struct pkg *pkg, struct archive *a __unused, struct archive_entry *ae,
- 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs)
+ 
    const char *path, struct pkg *local __unused, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	struct pkg_file *f;

	const struct stat *aest;
@@ -609,7 +613,7 @@ do_extract_hardlink(struct pkg *pkg, struct archive *a __unused, struct archive_
	lp = archive_entry_hardlink(ae);

	aest = archive_entry_stat(ae);
- 
	if (create_hardlink(pkg, f, lp, tempdirs) == EPKG_FATAL)
+ 
	if (create_hardlink(pkg, f, lp, tempdirs, symlinks_allowed) == EPKG_FATAL)

		return (EPKG_FATAL);



	metalog_add(PKG_METALOG_FILE, RELATIVE_PATH(path),
@@ -641,7 +645,8 @@ retry:


static int

create_regfile(struct pkg *pkg, struct pkg_file *f, struct archive *a,
- 
    struct archive_entry *ae, int fromfd, struct pkg *local, tempdirs_t *tempdirs)
+ 
    struct archive_entry *ae, int fromfd, struct pkg *local, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	int fd = -1;

	size_t len;
@@ -649,7 +654,7 @@ create_regfile(struct pkg *pkg, struct pkg_file *f, struct archive *a,
	char *path;

	struct tempdir *tmpdir = NULL;
- 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs);
+ 
	tmpdir = get_tempdir(pkg->rootfd, f->path, tempdirs, symlinks_allowed);

	if (tmpdir == NULL && errno == 0)

		hidden_tempfile(f->temppath, sizeof(f->temppath), f->path);
@@ -733,7 +738,7 @@ create_regfile(struct pkg *pkg, struct pkg_file *f, struct archive *a,


static int

do_extract_regfile(struct pkg *pkg, struct archive *a, struct archive_entry *ae,
- 
    const char *path, struct pkg *local, tempdirs_t *tempdirs)
+ 
    const char *path, struct pkg *local, tempdirs_t *tempdirs, stringlist_t *symlinks_allowed)

{

	struct pkg_file *f;

	const struct stat *aest;
@@ -753,7 +758,7 @@ do_extract_regfile(struct pkg *pkg, struct archive *a, struct archive_entry *ae,
	fill_timespec_buf(aest, f->time);

	archive_entry_fflags(ae, &f->fflags, &clear);
- 
	if (create_regfile(pkg, f, a, ae, -1, local, tempdirs) == EPKG_FATAL)
+ 
	if (create_regfile(pkg, f, a, ae, -1, local, tempdirs, symlinks_allowed) == EPKG_FATAL)

		return (EPKG_FATAL);



	metalog_add(PKG_METALOG_FILE, RELATIVE_PATH(path),
@@ -765,14 +770,15 @@ do_extract_regfile(struct pkg *pkg, struct archive *a, struct archive_entry *ae,


static int

do_extract(struct archive *a, struct archive_entry *ae,
- 
    int nfiles, struct pkg *pkg, struct pkg *local, tempdirs_t *tempdirs)
+ 
    int nfiles, struct pkg *pkg, struct pkg *local, tempdirs_t *tempdirs,
+ 
    stringlist_t *symlinks_allowed)

{

	int	retcode = EPKG_OK;

	int	ret = 0, cur_file = 0;

	char	path[MAXPATHLEN];

	int (*extract_cb)(struct pkg *pkg, struct archive *a,

	    struct archive_entry *ae, const char *path, struct pkg *local,
- 
	    tempdirs_t *tempdirs);
+ 
	    tempdirs_t *tempdirs, stringlist_t *sa);



#ifndef HAVE_ARC4RANDOM

	srand(time(NULL));
@@ -835,7 +841,7 @@ do_extract(struct archive *a, struct archive_entry *ae,
			break;

		}
- 
		if (extract_cb(pkg, a, ae, path, local, tempdirs) != EPKG_OK) {
+ 
		if (extract_cb(pkg, a, ae, path, local, tempdirs, symlinks_allowed) != EPKG_OK) {

			retcode = EPKG_FATAL;

			goto cleanup;

		}
@@ -1239,6 +1245,7 @@ pkg_add_common(struct pkgdb *db, const char *path, unsigned flags,
	int			 ret;

	int			 nfiles;

	tempdirs_t		 tempdirs = tll_init();
+ 
	stringlist_t		 symlinks_allowed = tll_init();



	assert(path != NULL);
@@ -1343,7 +1350,8 @@ pkg_add_common(struct pkgdb *db, const char *path, unsigned flags,
	 */

	if (extract) {

		pkg_register_cleanup_callback(pkg_rollback_cb, pkg);
- 
		retcode = do_extract(a, ae, nfiles, pkg, local, &tempdirs);
+ 
		tll_push_back(symlinks_allowed, pkg->prefix);
+ 
		retcode = do_extract(a, ae, nfiles, pkg, local, &tempdirs, &symlinks_allowed);

		pkg_unregister_cleanup_callback(pkg_rollback_cb, pkg);

		if (retcode != EPKG_OK) {

			/* If the add failed, clean up (silently) */
@@ -1496,6 +1504,8 @@ pkg_add_fromdir(struct pkg *pkg, const char *src)
	size_t link_len;

	bool install_as_user;

	tempdirs_t tempdirs = tll_init();
+ 
	stringlist_t symlinks_allowed = tll_init();
+ 
	tll_push_back(symlinks_allowed, pkg->prefix);



	install_as_user = (getenv("INSTALL_AS_USER") != NULL);
@@ -1551,7 +1561,7 @@ pkg_add_fromdir(struct pkg *pkg, const char *src)
#endif

#endif
- 
		if (create_dir(pkg, d, &tempdirs) == EPKG_FATAL) {
+ 
		if (create_dir(pkg, d, &tempdirs, &symlinks_allowed) == EPKG_FATAL) {

			retcode = EPKG_FATAL;

			goto cleanup;

		}
@@ -1623,7 +1633,7 @@ pkg_add_fromdir(struct pkg *pkg, const char *src)
				    "'%s'", f->path);

			}

			target[link_len] = '\0';
- 
			if (create_symlinks(pkg, f, target, &tempdirs) == EPKG_FATAL) {
+ 
			if (create_symlinks(pkg, f, target, &tempdirs, &symlinks_allowed) == EPKG_FATAL) {

				retcode = EPKG_FATAL;

				goto cleanup;

			}
@@ -1644,13 +1654,13 @@ pkg_add_fromdir(struct pkg *pkg, const char *src)
				}

			}

			if (path != NULL) {
- 
				if (create_hardlink(pkg, f, path, &tempdirs) == EPKG_FATAL) {
+ 
				if (create_hardlink(pkg, f, path, &tempdirs, &symlinks_allowed) == EPKG_FATAL) {

					close(fd);

					retcode = EPKG_FATAL;

					goto cleanup;

				}

			} else {
- 
				if (create_regfile(pkg, f, NULL, NULL, fd, NULL, &tempdirs) == EPKG_FATAL) {
+ 
				if (create_regfile(pkg, f, NULL, NULL, fd, NULL, &tempdirs, &symlinks_allowed) == EPKG_FATAL) {

					close(fd);

					retcode = EPKG_FATAL;

					goto cleanup;
@@ -1672,6 +1682,7 @@ pkg_add_fromdir(struct pkg *pkg, const char *src)
	retcode = pkg_extract_finalize(pkg, &tempdirs);



cleanup:
+ 
	tll_free(symlinks_allowed);

	tll_free_and_free(hardlinks, free);

	close(fromfd);

	return (retcode);
modified libpkg/private/pkg.h
@@ -122,7 +122,6 @@
#define DL_FREE(head, free_func) DL_FREE2(head, free_func, prev, next)



typedef tll(struct pkg_kv *) kvlist_t;
- 
typedef tll(char *) stringlist_t;



typedef enum {

	IPALL = 0,
modified libpkg/private/utils.h
@@ -41,6 +41,8 @@
#define STARTS_WITH(string, needle) (strncasecmp(string, needle, strlen(needle)) == 0)

#define RELATIVE_PATH(p) (p + (*p == '/' ? 1 : 0))
+ 
typedef tll(char *) stringlist_t;
+ 


#define ERROR_SQLITE(db, query) do { \

	pkg_emit_error("sqlite error while executing %s in file %s:%d: %s", query, \

	__FILE__, __LINE__, sqlite3_errmsg(db)); \
@@ -114,7 +116,7 @@ char *rtrimspace(char *buf);
void hidden_tempfile(char *buf, int buflen, const char *path);

void append_random_suffix(char *buf, int buflen, int suffixlen);

char *json_escape(const char *str);
- 
struct tempdir *open_tempdir(int rootfd, const char *path);
+ 
struct tempdir *open_tempdir(int rootfd, const char *path, stringlist_t *strlist);

const char *get_http_auth(void);



#endif
modified libpkg/utils.c
@@ -968,7 +968,7 @@ json_escape(const char *str)
}



struct tempdir *
- 
open_tempdir(int rootfd, const char *path)
+ 
open_tempdir(int rootfd, const char *path, stringlist_t *symlinks_allowed)

{

	struct stat st;

	char walk[MAXPATHLEN];
@@ -980,12 +980,19 @@ open_tempdir(int rootfd, const char *path)
	while ((dir = strrchr(walk, '/')) != NULL) {

		*dir = '\0';

		cnt++;
- 
		/* accept symlinks pointing to directories */
+ 
		/* accept symlinks pointing to directories only for prefix */

		len = strlen(walk);

		if (len == 0 && cnt == 1)

			break;

		if (len > 0) {
- 
			if (fstatat(rootfd, RELATIVE_PATH(walk), &st, 0) == -1)
+ 
			int flag = AT_SYMLINK_NOFOLLOW;
+ 
			if (symlinks_allowed != NULL) {
+ 
				tll_foreach(*symlinks_allowed, t) {
+ 
					if (strcmp(RELATIVE_PATH(walk), RELATIVE_PATH(t->item)) == 0)
+ 
						flag = 0;
+ 
				}
+ 
			}
+ 
			if (fstatat(rootfd, RELATIVE_PATH(walk), &st, flag) == -1)

				continue;

			if (S_ISDIR(st.st_mode) && cnt == 1)

				break;
modified tests/frontend/upgrade.sh
@@ -8,7 +8,8 @@ tests_init \
	three_digit_revision \

	dual_conflict \

	file_become_dir \
- 
	dir_become_file
+ 
	dir_become_file \
+ 
	dir_is_symlink_to_a_dir



issue1881_body() {

	atf_check -s exit:0 sh ${RESOURCEDIR}/test_subr.sh new_pkg pkg1 pkg_a 1
@@ -262,3 +263,24 @@ dir_become_file_body() {
	atf_check pkg create -M pkg.ucl -p plist-2

	atf_check -o ignore pkg -o REPOS_DIR="${TMPDIR}" -r ${TMPDIR}/target install -Uy ${TMPDIR}/pkg-2.pkg

}
+
+ 
dir_is_symlink_to_a_dir_body()
+ 
{
+ 
	atf_check -s exit:0 sh ${RESOURCEDIR}/test_subr.sh new_pkg "pkg" "pkg" "1"
+ 
	mkdir share lib lib/something
+ 
	ln -sf ../lib/something share/something
+ 
	echo "entry" > lib/something/file
+ 
	echo "${TMPDIR}/lib/something/file" > plist-1
+ 
	echo "${TMPDIR}/share/something" >> plist-1
+ 
	atf_check pkg create -M pkg.ucl -p plist-1
+ 
	mkdir target
+ 
	atf_check -o ignore pkg -o REPOS_DIR="${TMPDIR}" -r ${TMPDIR}/target install -Uy ${TMPDIR}/pkg-1.pkg
+ 
	atf_check -s exit:0 sh ${RESOURCEDIR}/test_subr.sh new_pkg "pkg" "pkg" "2"
+ 
	rm share/something
+ 
	mkdir share/something
+ 
	echo "entry" > share/something/file
+ 
	echo "${TMPDIR}/lib/something/file" > plist-2
+ 
	echo "${TMPDIR}/share/something/file" >> plist-2
+ 
	atf_check pkg create -M pkg.ucl -p plist-2
+ 
	atf_check -o ignore pkg -o REPOS_DIR="${TMPDIR}" -r ${TMPDIR}/target install -Uy ${TMPDIR}/pkg-2.pkg
+ 
}
modified tests/lib/utils.c
@@ -75,10 +75,10 @@ ATF_TC_BODY(open_tempdir, tc) {
	struct tempdir *t;

	int rootfd = open(getenv("TMPDIR"), O_DIRECTORY);

	ATF_REQUIRE_MSG(rootfd  != -1, "impossible to open TMPDIR");
- 
	t = open_tempdir(rootfd, "/plop");
+ 
	t = open_tempdir(rootfd, "/plop", NULL);

	ATF_REQUIRE(t == NULL);

	mkdirat(rootfd, "usr", 0755);
- 
	t = open_tempdir(rootfd, "/usr/local/directory");
+ 
	t = open_tempdir(rootfd, "/usr/local/directory", NULL);

	ATF_REQUIRE(t != NULL);

	ATF_REQUIRE_STREQ(t->name, "/usr/local");

	ATF_REQUIRE_EQ(t->len, strlen("/usr/local"));
@@ -86,7 +86,7 @@ ATF_TC_BODY(open_tempdir, tc) {
	ATF_REQUIRE(t->fd != -1);

	close(t->fd);

	free(t);
- 
	t = open_tempdir(rootfd, "/nousr/local/directory");
+ 
	t = open_tempdir(rootfd, "/nousr/local/directory", NULL);

	ATF_REQUIRE(t != NULL);

	ATF_REQUIRE_STREQ(t->name, "/nousr");

	ATF_REQUIRE_EQ(t->len, strlen("/nousr"));
@@ -97,7 +97,7 @@ ATF_TC_BODY(open_tempdir, tc) {
	mkdirat(rootfd, "dir", 0755);

	/* a file in the path */

	close(openat(rootfd, "dir/file1", O_CREAT|O_WRONLY, 0644));
- 
	t = open_tempdir(rootfd, "/dir/file1/test");
+ 
	t = open_tempdir(rootfd, "/dir/file1/test", NULL);

	ATF_REQUIRE(t != NULL);

	ATF_REQUIRE_STREQ(t->name, "/dir/file1");

	ATF_REQUIRE_EQ(t->len, strlen("/dir/file1"));