Summary: | net-vpn/peervpn: root privilege escalation via "chown -R" in pkg_preinst | ||
---|---|---|---|
Product: | Gentoo Security | Reporter: | Michael Orlitzky <mjo> |
Component: | Auditing | Assignee: | Gentoo Security <security> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | mgorny, security-audit, zmedico |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- |
Description
Michael Orlitzky
2017-09-14 12:33:26 UTC
How about his: pkg_preinst() { if ! has_version '>=net-vpn/peervpn-0.044-r5' && \ [[ -d ${EROOT}etc/${PN} && $(find "${EROOT}etc/peervpn" -user "${PN}" ! -type l -print) ]]; then ewarn "Tightening '${EROOT}etc/${PN}' permissions for bug 629418" while read -r -d ''; do chown root:${PN} "${REPLY}" || die chmod g+rX-w,o-rwx "${REPLY}" || die done < <(find "${EROOT}etc/peervpn" -user "${PN}" ! -type l -print0) fi } (In reply to Zac Medico from comment #1) > How about his: That's the idea, but there's a race condition you have to worry about. The first time around, "chown --from=..." is necessary because "find" might return a path that the "peervpn" user can overwrite before you have time to call chown on it. I think chmod has the same issue, but once you've chown'ed everything (if you don't find any "peervpn"-owned paths), the chmods should be immune because he can't replace anything. How about this: pkg_preinst() { if ! has_version '>=net-vpn/peervpn-0.044-r4' && \ [[ -d ${EROOT}etc/${PN} && ! -L ${EROOT}etc/${PN} && $(find "${EROOT}etc/${PN}" -maxdepth 1 \ -user "${PN}" ! -type l -print) ]]; then ewarn "Tightening '${EROOT}etc/${PN}' permissions for bug 629418" # Tighten the parent directory permissions first, in # order to protect against race conditions involving a # less-privileged user. chown root:${PN} "${EROOT}etc/${PN}" chmod g+rX-w,o-rwx "${EROOT}etc/${PN}" # Don't chown/chmod the referent of a symlink # owned by a less-privileged user. while read -r -d ''; do chown root:${PN} "${REPLY}" || die chmod g+rX-w,o-rwx "${REPLY}" || die done < <(find "${EROOT}etc/${PN}" -mindepth 1 -maxdepth 1 \ -user "${PN}" ! -type l -print0) fi } (In reply to Zac Medico from comment #3) > How about this: I think this one just might be safe; I can't figure out a way to trick it. The mindepth/maxdepth should ensure that you're only operating in /etc/${PN}, which you fix first... Unrestricting and reassigning to security@ per bug #705894 unrestricting per bug 705894 (In reply to Zac Medico from comment #3) > How about this: > > pkg_preinst() { > if ! has_version '>=net-vpn/peervpn-0.044-r4' && \ > [[ -d ${EROOT}etc/${PN} && ! -L ${EROOT}etc/${PN} && > $(find "${EROOT}etc/${PN}" -maxdepth 1 \ > -user "${PN}" ! -type l -print) ]]; then > ewarn "Tightening '${EROOT}etc/${PN}' permissions for bug 629418" > # Tighten the parent directory permissions first, in > # order to protect against race conditions involving a > # less-privileged user. > chown root:${PN} "${EROOT}etc/${PN}" > chmod g+rX-w,o-rwx "${EROOT}etc/${PN}" > # Don't chown/chmod the referent of a symlink > # owned by a less-privileged user. > while read -r -d ''; do > chown root:${PN} "${REPLY}" || die > chmod g+rX-w,o-rwx "${REPLY}" || die > done < <(find "${EROOT}etc/${PN}" -mindepth 1 -maxdepth 1 \ > -user "${PN}" ! -type l -print0) > fi > } Zac, is this good now? I don't see the mindepth parts in the ebuild... The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=41725d13927f9012e1758ef662f3e5ba351423ac commit 41725d13927f9012e1758ef662f3e5ba351423ac Author: Zac Medico <zmedico@gentoo.org> AuthorDate: 2020-05-04 04:00:28 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2020-05-04 04:02:01 +0000 net-vpn/peervpn: 0.044-r5 revbump for bug 630972 Tighten up permission adjustments related to bug 629418. Bug: https://bugs.gentoo.org/630972 Package-Manager: Portage-2.3.99, Repoman-2.3.22 Signed-off-by: Zac Medico <zmedico@gentoo.org> ...eervpn-0.044-r4.ebuild => peervpn-0.044-r5.ebuild} | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) 0.044-r5 has the pkg_preinst shown in comment #3. Removing it now. |