The pkg_config phase for the dev-db/postgresql ebuilds calls "chown -R" on the DATA_DIR after verifying that DATA_DIR is empty: if [ -n "$(ls -A ${DATA_DIR} 2> /dev/null)" ] ; then eerror "The given directory, '${DATA_DIR}', is not empty." eerror "Modify DATA_DIR to point to an empty directory." die "${DATA_DIR} is not empty." fi einfo "Creating the data directory ..." if [[ ${EUID} == 0 ]] ; then mkdir -p "${DATA_DIR}" chown -Rf postgres:postgres "${DATA_DIR}" chmod 0700 "${DATA_DIR}" fi If the DATA_DIR is actually empty, then the "-R" is harmless but also pointless. So right away, one possible solution is to remove the "-R". However, if the DATA_DIR is not empty, then the "postgres" user can exploit the "chown -R" call to become root. That's where the race condition comes in: there is a brief period of time between where "ls" is called and where "chown -R" is called in which the "postgres" user can place a hard link in DATA_DIR. If he manages to create the hard link between "ls" and "chown" calls, then "chown -R" will hit the hard link, and give ownership of the target to the "postgres" user. That in turn can be used to gain root. Later on in the ebuild, "su postgres" is used to do some stuff. Maybe the DATA_DIR can be created (and chmod'ed) using "su postgres" to avoid the chown call?
commit 850efe2a5700c2ba30f9e9860dd83143cf15da34 (HEAD -> master, origin/master, origin/HEAD) Author: Aaron W. Swenson <titanofold@gentoo.org> Date: Sun Feb 11 10:54:10 2018 -0500 dev-db/postgresql: Cleanup Old and Insecure Files Bug: https://bugs.gentoo.org/627462 Bug: https://bugs.gentoo.org/636978 Bug: https://bugs.gentoo.org/630824 Bug: https://bugs.gentoo.org/603720 Bug: https://bugs.gentoo.org/603716 Package-Manager: Portage-2.3.19, Repoman-2.3.6 commit 3b3ec30d0b02920ec76eeef4db2a968c3a907d23 Author: Jeroen Roovers <jer@gentoo.org> Date: Sun Feb 11 13:09:46 2018 +0100 dev-db/postgresql: Stable for HPPA too. Package-Manager: Portage-2.3.24, Repoman-2.3.6 RepoMan-Options: --ignore-arches
Eh, so I didn't really fix this. I don't know why I referenced it. So here's what I think I could do: if [[ ${EUID} == 0 ]] ; then mkdir -p "$(dirname ${DATA_DIR})" $tmpdir = `mktemp -d` || die "Couldn't make temporary directory" chown postgres:postgres $tmpdir mv -T $tmpdir "${DATA_DIR%/}" || die elsif # other things fi We don't care what happens to the parent director{y,ies}. We just care that they're there. mktemp will create a directory with a random name that belongs to root, and it'll be created with mode 0700. We can change the permissions on that tmpdir while it's there before trying to move it to the desired location. If the DATA_DIR exists and is nonempty, mv will fail. If DATA_DIR is a link, mv will fail. If the DATA_DIR exists, is a directory, but empty, mv will succeed replacing the DATA_DIR with the one with the proper permissions. The only problem is BSD doesn't support -T on mv. So, that's as close as I've gotten to making sure we always win this race.
(In reply to Aaron W. Swenson from comment #2) > $tmpdir = `mktemp -d` || die "Couldn't make temporary directory" > chown postgres:postgres $tmpdir Since you're already giving the postgres user a shell, you could "su -c ... postgres" here to avoid the chown.
(In reply to Michael Orlitzky from comment #3) > (In reply to Aaron W. Swenson from comment #2) > > $tmpdir = `mktemp -d` || die "Couldn't make temporary directory" > > chown postgres:postgres $tmpdir > > Since you're already giving the postgres user a shell, you could "su -c ... > postgres" here to avoid the chown. Yes, I could. That would save a line, maybe. And, I could probably do the mv without the -T and instead just give the -n(o clobber) option to it. If someone nefarious were to race the script and have swapped out $(dirname $DATA_DIR) or put a symlink at DATA_DIR, the worst that would happen is a directory is created inside that location.
All right, I think this is the best I can do: ebegin "Creating the data directory" local tmpdir if [[ ${EUID} == 0 ]] ; then tmpdir=$(su postgres -c 'mktemp -d') \ || die "Couldn't make temporary directory" mkdir -p "$(dirname ${DATA_DIR})" mv -n "${tmpdir}" "${DATA_DIR%/}" if [[ -d ${tmpdir} ]] ; then eend 1 rmdir "${tmpdir}" die "Couldn't move temporary directory to destination" else eend 0 fi else mkdir -p "${DATA_DIR}" if [[ -d ${DATA_DIR} ]] ; then eend 0 else eend 1 die "Couldn't make DATA_DIR" fi fi Only when the ebuild is running as root, do we try to make sure everything is exclusive to the postgres system user. We start by making a temporary directory as postgres so that the directory starts out owned by postgres and with permission 0700. We don't care about ownership about parent directories; we just care that they are present. Because "mv -n" pretty much always returns 0 even if it didn't move anything, we check if the temporary directory still exists and die if it does. For the non-root case, everything is owned and run as that non-root user. We just need to be sure the directory was made. When initdb is executed in the next step, initdb will ensure the DATA_DIR is empty. If non-empty, it bails.
And, after digging through this a bit more this whole exercise may have been pointless. initdb/PostgreSQL has it's own implementation of "mkdir -p" that creates/sets permissions and checks things at each step. The real answer seems to be to just scrap this section altogether, and let initdb handle everything. In fact, it seems it's been redundant since 2003. pg_mkdir_p: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/port/pgmkdirp.c;h=d943559760d8980a1582f0bee064c0bea52b5597;hb=HEAD initdb: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/initdb/initdb.c;h=ae22e7d9fb89db67d187aa2e43cfe5db42011d9d;hb=HEAD#l2759
Unrestricting and reassigning to security@ per bug #705894
unrestricting per bug 705894
(In reply to Aaron W. Swenson from comment #6) > And, after digging through this a bit more this whole exercise may have been > pointless. > > initdb/PostgreSQL has it's own implementation of "mkdir -p" that > creates/sets permissions and checks things at each step. > > The real answer seems to be to just scrap this section altogether, and let > initdb handle everything. In fact, it seems it's been redundant since 2003. > > pg_mkdir_p: > https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/port/ > pgmkdirp.c;h=d943559760d8980a1582f0bee064c0bea52b5597;hb=HEAD > > initdb: > https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/initdb/ > initdb.c;h=ae22e7d9fb89db67d187aa2e43cfe5db42011d9d;hb=HEAD#l2759 Do you know when this was first fixed? Anyway, this is obsolete now.
