Mercurial > p > roundup > code
changeset 1685:b6621f8bd496 maint-0.5
backported XSS message cleaning fix [SF#757128]
| author | Richard Jones <richard@users.sourceforge.net> |
|---|---|
| date | Tue, 24 Jun 2003 03:33:56 +0000 |
| parents | 5127c4560f1e |
| children | 580ce488f976 |
| files | CHANGES.txt roundup/cgi/client.py test/test_cgi.py |
| diffstat | 3 files changed, 39 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Tue Jun 24 03:09:35 2003 +0000 +++ b/CHANGES.txt Tue Jun 24 03:33:56 2003 +0000 @@ -1,6 +1,9 @@ This file contains the changes to the Roundup system over time. The entries are given with the most recent entry first. +2003-07-?? 0.5.9 +- backported XSS message cleaning fix (sf bug 757128) + 2003-06-19 0.5.8 - plugged cross-site-scripting hole (thanks Jeff Epler)
--- a/roundup/cgi/client.py Tue Jun 24 03:09:35 2003 +0000 +++ b/roundup/cgi/client.py Tue Jun 24 03:33:56 2003 +0000 @@ -1,4 +1,4 @@ -# $Id: client.py,v 1.65.2.9 2003-06-19 23:02:32 richard Exp $ +# $Id: client.py,v 1.65.2.10 2003-06-24 03:33:56 richard Exp $ __doc__ = """ WWW request handler (also used in the stand-alone server). @@ -47,10 +47,16 @@ description="User may manipulate user Roles through the web") security.addPermissionToRole('Admin', p) -def clean_message(match, ok={'a':1,'i':1,'b':1,'br':1}): +# used to clean messages passed through CGI variables - HTML-escape any tag +# that isn't <a href="">, <i>, <b> and <br> (including XHTML variants) so +# that people can't pass through nasties like <script>, <iframe>, ... +CLEAN_MESSAGE_RE = r'(<(/?(.*?)(\s*href="[^"]")?\s*/?)>)' +def clean_message(message, mc=re.compile(CLEAN_MESSAGE_RE, re.I)): + return mc.sub(clean_message_callback, message) +def clean_message_callback(match, ok={'a':1,'i':1,'b':1,'br':1}): ''' Strip all non <a>,<i>,<b> and <br> tags from a string ''' - if ok.has_key(match.group(2)): + if ok.has_key(match.group(3).lower()): return match.group(1) return '<%s>'%match.group(2) @@ -256,8 +262,7 @@ # reopen the database as the correct user self.opendb(self.user) - def determine_context(self, dre=re.compile(r'([^\d]+)(\d+)'), - mc=re.compile(r'(</?(.*?)>)')): + def determine_context(self, dre=re.compile(r'([^\d]+)(\d+)')): ''' Determine the context of this page from the URL: The URL path after the instance identifier is examined. The path @@ -339,10 +344,10 @@ # see if we were passed in a message if self.form.has_key(':ok_message'): - msg = mc.sub(clean_message, self.form[':ok_message'].value) + msg = clean_message(self.form[':ok_message'].value) self.ok_message.append(msg) if self.form.has_key(':error_message'): - msg = mc.sub(clean_message, self.form[':error_message'].value) + msg = clean_message(self.form[':error_message'].value) self.error_message.append(msg) def serve_file(self, designator, dre=re.compile(r'([^\d]+)(\d+)')):
--- a/test/test_cgi.py Tue Jun 24 03:09:35 2003 +0000 +++ b/test/test_cgi.py Tue Jun 24 03:33:56 2003 +0000 @@ -8,7 +8,7 @@ # but WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # -# $Id: test_cgi.py,v 1.4.2.2 2003-03-21 21:43:04 richard Exp $ +# $Id: test_cgi.py,v 1.4.2.3 2003-06-24 03:33:56 richard Exp $ import unittest, os, shutil, errno, sys, difflib, cgi @@ -24,6 +24,26 @@ form.list.append(cgi.MiniFieldStorage(k, v)) return form +cm = client.clean_message +class MessageTestCase(unittest.TestCase): + def testCleanMessageOK(self): + self.assertEqual(cm('<br>x<br />'), '<br>x<br />') + self.assertEqual(cm('<i>x</i>'), '<i>x</i>') + self.assertEqual(cm('<b>x</b>'), '<b>x</b>') + self.assertEqual(cm('<a href="y">x</a>'), + '<a href="y">x</a>') + self.assertEqual(cm('<BR>x<BR />'), '<BR>x<BR />') + self.assertEqual(cm('<I>x</I>'), '<I>x</I>') + self.assertEqual(cm('<B>x</B>'), '<B>x</B>') + self.assertEqual(cm('<A HREF="y">x</A>'), + '<A HREF="y">x</A>') + + def testCleanMessageBAD(self): + self.assertEqual(cm('<script>x</script>'), + '<script>x</script>') + self.assertEqual(cm('<iframe>x</iframe>'), + '<iframe>x</iframe>') + class FormTestCase(unittest.TestCase): def setUp(self): self.dirname = '_test_cgi_form' @@ -277,7 +297,9 @@ def suite(): - l = [unittest.makeSuite(FormTestCase), + l = [ + unittest.makeSuite(FormTestCase), + unittest.makeSuite(MessageTestCase), ] return unittest.TestSuite(l)