The shell command invoked for sending mail is:
/bin/mail -s "CVS: %(SUBJECT)s" %(PEOPLE)s 2>&1 > /dev/null
SUBJECT is determined from the file name, version number, and tags.
Are you entirely sure that the file name, version number, and tags, couldn't
contain a string like:
`rm -rf /`
or:
";cat /etc/passwd
?
I'm not entirely sure of that. Anyway, with my patch for subject lines derived
from log entries (in another message), it is *definitely* possible that the
SUBJECT string could have such stuff. My fix is to change the command to:
/bin/mail -s 'CVS: %(SUBJECT)s' %(PEOPLE)s 2>&1 > /dev/null
And to run SUBJECT through:
SUBJECT = string.replace(SUBJECT, "'", '"')
My reading of the bash 2.05 man page tells me that this should render the
contents of SUBJECT inert when used in that command. I hope the same applies to
Bourne shell.
This much was already in the "subject lines from log entries" patch that
I already submitted, but I subsequently went hunting for similar problems and
appended is a patch which ensures that no other such sneakiness could get in
to the other commands that syncmail invokes. As a bonus, it also makes syncmail
handle filenames with spaces (and all other kinds of weird character, except
for '\').
Regards,
Zooko
---
zooko.com
Security and Distributed Systems Engineering
---
Index: syncmail
===================================================================
RCS file: /cvsroot/mnet/CVSROOT/syncmail,v
retrieving revision 1.14
retrieving revision 1.17
diff -u -r1.14 -r1.17
--- syncmail 17 Mar 2002 14:26:25 -0000 1.14
+++ syncmail 17 Mar 2002 15:17:03 -0000 1.17
@@ -61,11 +61,13 @@
"""
+# standard Python modules
+import getopt
import os
-import sys
+import re
import string
+import sys
import time
-import getopt
# Notification command
MAILCMD = "/bin/mail -s 'CVS: %(SUBJECT)s' %(PEOPLE)s 2>&1 > /dev/null"
@@ -90,9 +92,29 @@
+REV_RE=re.compile("^[0-9.]+$")
def calculate_diff(filespec, contextlines):
try:
file, oldrev, newrev = string.split(filespec, ',')
+ if not REV_RE.match(oldrev):
+ raise ValueError
+ if not REV_RE.match(newrev):
+ raise ValueError
+ if string.find(file, '\\') != -1:
+ # I'm sorry, a file name that contains a backslash is just too much.
+ # XXX if someone wants to figure out how to escape the backslashes in a safe way to allow filenames containing backslashes, this is the place to do it. --Zooko 2002-03-17
+ raise ValueError
+ if string.find(file, "'") != -1:
+ # Those crazy users put single-quotes in their file names!
+ # Now we have to escape everything that is meaningful inside double-quotes.
+ filestr = string.replace(file, '`', '\`')
+ filestr = string.replace(filestr, '"', '\"')
+ filestr = string.replace(filestr, '$', '\$')
+ # and quote it with double-quotes.
+ filestr = '"' + filestr + '"'
+ else:
+ # quote it with single-quotes.
+ filestr = "'" + file + "'"
except ValueError:
# No diff to report
return '***** Bogus filespec: %s' % filespec
@@ -101,7 +123,7 @@
if os.path.exists(file):
fp = open(file)
else:
- update_cmd = 'cvs -fn update -r %s -p %s' % (newrev, file)
+ update_cmd = "cvs -fn update -r %s -p %s" % (newrev, filestr)
fp = os.popen(update_cmd)
lines = fp.readlines()
fp.close()
@@ -125,8 +147,7 @@
difftype = "-C " + str(contextlines)
else:
difftype = "-u"
- diffcmd = "/usr/bin/cvs -f diff -kk %s --minimal -r %s -r %s '%s'" % (
- difftype, oldrev, newrev, file)
+ diffcmd = "/usr/bin/cvs -f diff -kk %s --minimal -r %s -r %s %s" % (difftype, oldrev, newrev, filestr)
fp = os.popen(diffcmd)
lines = fp.readlines()
sts = fp.close()
|
