Hi, i just committed the follwing to the OpenBSD ports tree, where i'm maintaining a port of groff-1.21.
This posting includes a commit message and a patch at the end. Even though the audit by Solar Designer did not, unless i missed something, reveal any vulnerabilities that persist in groff-1.21, i think many of his patches should be considered from the point of view of a proactive stance on security: Don't just fix what is proven exploitable, rather fix everything that is buggy or even potentially dangerous. In particular, in pdfroff.sh, using mktemp -d for security reasons, then falling back to something else (which is very unsafe) in case of failure looks dangerous. At the very least, you should use many XXXXXXXXXXs and not just the $$ when falling back. So i fixed that one even in our port. All the rest is not worth patches in a port, but should, i think, all the same be considered upstream. Yours, Ingo ----- Forwarded message from <[email protected]> ----- From: Ingo Schwarze <[email protected]> Date: Thu, 23 Jun 2011 06:14:51 -0600 (MDT) To: [email protected] Subject: CVS: cvs.openbsd.org: ports CVSROOT: /cvs Module name: ports Changes by: [email protected] 2011/06/23 06:14:51 Modified files: textproc/groff : Makefile Added files: textproc/groff/patches: patch-contrib_pdfmark_pdfroff_sh Log message: Following http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=538330, Solar Designer did an audit of temp file handling in groff-1.20. He found and fixed *lots* of ugliness, but most does not look exploitable and some was already improved in groff-1.21. This is my own fix for the only one that, with a huge amount of extra paranoia, might be worth patching. To mount an exploit, the attacker would need to trick root into setting an unusable TMPDIR (or similar) variable in the environment such that mktemp -d fails, then convince root to run pdfroff (*you* don't run that as root, do you?), then handle a race condition to find the PID and predict the temp file name to mount a symlink attack. "I think we should still go for the fix" jasper@ ----- End forwarded message ----- --- contrib/pdfmark/pdfroff.sh.orig Fri Dec 31 08:33:09 2010 +++ contrib/pdfmark/pdfroff.sh Wed Jun 22 01:37:47 2011 @@ -153,11 +153,10 @@ else # # Creation of a private temporary directory was unsuccessful; - # fall back to user nominated directory, (using current directory - # as default), and schedule removal of only the temporary files. - # - GROFF_TMPDIR=${TMPDIR} - trap "rm -f ${GROFF_TMPDIR}/pdf$$.*" 0 + # DO NOT fall back to user nominated directory, + # because that would allow symlink attacks. + echo >&2 "$CMD: mktemp(1) -d failure" + exit 1 fi # # In the case of abnormal termination events, we force an exit _______________________________________________ bug-groff mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-groff
