Dragon wrote:
>Mark Sapiro sent the message below at 12:41 PM 3/11/2007:
>
>>It's as I suspected. The various input tags on the Membership list look
>>like
>>
>><INPUT name="[EMAIL PROTECTED]" type="CHECKBOX" value="off" >
>>
>>where [EMAIL PROTECTED] is the email address. Clearly, if the address
>>contains double quotes, the field name gets truncated or garbled, so
>>it isn't possible to change anything for this member from the
>>Membership list page..
>---------------- End original message. ---------------------
>
>Which is valid, and proper HTML usage, all parameters in any HTML tag
>should be enclosed in quotes. It is mandatory in XHTML.
I never meant to imply that this was not proper usage. I only quoted
the input tag in order to show that the email address was part of a
field name.
>So how do you deal with this?
>
>Quite simply by escaping any non alpha-numeric character with either
>its symbolic or numeric code. It is always good practice in dealing
>with any sort of CGI or user generated data to ensure that just such
>situations or worse will not occur.
>
>If there is a Python module out there for escaping HTML strings, it
>seems like it would be a fairly simple task to apply the escape
>function while generating the output to the page.
That's half (or less) of the solution. The other part is recognizing
the input field names with escaped characters. The change to the
admin.py script is not that difficult. In case anyone is interested, I
have attached a patch.
I am reluctant to commit the patch for a few reasons. It breaks a
couple of screen scraping mailman-subscribers.py scripts which can be
fixed, but what else does it break?
Also, I'm not sure that there's really a problem needing to be fixed.
RFC 2821 allows quoted local-parts in email addresses, but are there
any in practice? If someone inadvertently subscribes an address with
quotes as the OP did, the bad address can be easily removed via
mass-remove even though not by the unsub checkbox.
I understand the point about good practice, and we do try to validate
user input in Mailman to avoid possible XSS attacks via the web
interface. What we're dealing with here are syntactically validated
email addresses so the really nasty stuff has already been caught.
Still, I'm interested in feedback from anyone who has an opinion about
this.
--
Mark Sapiro <[EMAIL PROTECTED]> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan
Index: admin.py
===================================================================
--- admin.py (revision 8160)
+++ admin.py (working copy)
@@ -1,4 +1,4 @@
-# Copyright (C) 1998-2006 by the Free Software Foundation, Inc.
+# Copyright (C) 1998-2007 by the Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
@@ -982,15 +982,16 @@
}
# Now populate the rows
for addr in members:
+ qaddr = urllib.quote(addr)
link = Link(mlist.GetOptionsURL(addr, obscure=1),
mlist.getMemberCPAddress(addr))
fullname = Utils.uncanonstr(mlist.getMemberName(addr),
mlist.preferred_language)
- name = TextBox(addr + '_realname', fullname, size=longest).Format()
- cells = [Center(CheckBox(addr + '_unsub', 'off', 0).Format()),
+ name = TextBox(qaddr + '_realname', fullname, size=longest).Format()
+ cells = [Center(CheckBox(qaddr + '_unsub', 'off', 0).Format()),
link.Format() + '<br>' +
name +
- Hidden('user', urllib.quote(addr)).Format(),
+ Hidden('user', qaddr).Format(),
]
# Do the `mod' option
if mlist.getMemberOption(addr, mm_cfg.Moderate):
@@ -999,7 +1000,7 @@
else:
value = 'off'
checked = 0
- box = CheckBox('%s_mod' % addr, value, checked)
+ box = CheckBox('%s_mod' % qaddr, value, checked)
cells.append(Center(box).Format())
for opt in ('hide', 'nomail', 'ack', 'notmetoo', 'nodupes'):
extra = ''
@@ -1018,23 +1019,23 @@
else:
value = 'off'
checked = 0
- box = CheckBox('%s_%s' % (addr, opt), value, checked)
+ box = CheckBox('%s_%s' % (qaddr, opt), value, checked)
cells.append(Center(box.Format() + extra))
# This code is less efficient than the original which did a has_key on
# the underlying dictionary attribute. This version is slower and
# less memory efficient. It points to a new MemberAdaptor interface
# method.
if addr in mlist.getRegularMemberKeys():
- cells.append(Center(CheckBox(addr + '_digest', 'off', 0).Format()))
+ cells.append(Center(CheckBox(qaddr + '_digest', 'off',
0).Format()))
else:
- cells.append(Center(CheckBox(addr + '_digest', 'on', 1).Format()))
+ cells.append(Center(CheckBox(qaddr + '_digest', 'on', 1).Format()))
if mlist.getMemberOption(addr, mm_cfg.OPTINFO['plain']):
value = 'on'
checked = 1
else:
value = 'off'
checked = 0
- cells.append(Center(CheckBox('%s_plain' % addr, value, checked)))
+ cells.append(Center(CheckBox('%s_plain' % qaddr, value, checked)))
# User's preferred language
langpref = mlist.getMemberLanguage(addr)
langs = mlist.GetAvailableLanguages()
@@ -1043,7 +1044,7 @@
selected = langs.index(langpref)
except ValueError:
selected = 0
- cells.append(Center(SelectOptions(addr + '_language', langs,
+ cells.append(Center(SelectOptions(qaddr + '_language', langs,
langdescs, selected)).Format())
usertable.AddRow(cells)
# Add the usertable and a legend
@@ -1427,7 +1428,8 @@
errors = []
removes = []
for user in users:
- if cgidata.has_key('%s_unsub' % user):
+ quser = urllib.quote(user)
+ if cgidata.has_key('%s_unsub' % quser):
try:
mlist.ApprovedDeleteMember(user, whence='member mgt page')
removes.append(user)
@@ -1438,7 +1440,7 @@
doc.addError(_('Ignoring changes to deleted member: %(user)s'),
tag=_('Warning: '))
continue
- value = cgidata.has_key('%s_digest' % user)
+ value = cgidata.has_key('%s_digest' % quser)
try:
mlist.setMemberOption(user, mm_cfg.Digests, value)
except (Errors.AlreadyReceivingDigests,
@@ -1448,28 +1450,28 @@
# BAW: Hmm...
pass
- newname = cgidata.getvalue(user+'_realname', '')
+ newname = cgidata.getvalue(quser+'_realname', '')
newname = Utils.canonstr(newname, mlist.preferred_language)
mlist.setMemberName(user, newname)
- newlang = cgidata.getvalue(user+'_language')
+ newlang = cgidata.getvalue(quser+'_language')
oldlang = mlist.getMemberLanguage(user)
if Utils.IsLanguage(newlang) and newlang <> oldlang:
mlist.setMemberLanguage(user, newlang)
- moderate = not not cgidata.getvalue(user+'_mod')
+ moderate = not not cgidata.getvalue(quser+'_mod')
mlist.setMemberOption(user, mm_cfg.Moderate, moderate)
# Set the `nomail' flag, but only if the user isn't already
# disabled (otherwise we might change BYUSER into BYADMIN).
- if cgidata.has_key('%s_nomail' % user):
+ if cgidata.has_key('%s_nomail' % quser):
if mlist.getDeliveryStatus(user) == MemberAdaptor.ENABLED:
mlist.setDeliveryStatus(user, MemberAdaptor.BYADMIN)
else:
mlist.setDeliveryStatus(user, MemberAdaptor.ENABLED)
for opt in ('hide', 'ack', 'notmetoo', 'nodupes', 'plain'):
opt_code = mm_cfg.OPTINFO[opt]
- if cgidata.has_key('%s_%s' % (user, opt)):
+ if cgidata.has_key('%s_%s' % (quser, opt)):
mlist.setMemberOption(user, opt_code, 1)
else:
mlist.setMemberOption(user, opt_code, 0)
------------------------------------------------------
Mailman-Users mailing list
Mailman-Users@python.org
http://mail.python.org/mailman/listinfo/mailman-users
Mailman FAQ: http://www.python.org/cgi-bin/faqw-mm.py
Searchable Archives: http://www.mail-archive.com/mailman-users%40python.org/
Unsubscribe:
http://mail.python.org/mailman/options/mailman-users/archive%40jab.org
Security Policy:
http://www.python.org/cgi-bin/faqw-mm.py?req=show&file=faq01.027.htp
