From 9351e5ff1c60394a16ac87690b08abee8e404a32 Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Sun, 12 Feb 2017 17:01:30 -0800
Subject: Implemented stricter listname validation.

---
 src/mailman/app/lifecycle.py            | 16 +++++++++++++++-
 src/mailman/app/tests/test_lifecycle.py |  9 ++++++++-
 src/mailman/docs/NEWS.rst               |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py
index 79ab20c8a..cfa01531b 100644
--- a/src/mailman/app/lifecycle.py
+++ b/src/mailman/app/lifecycle.py
@@ -17,12 +17,14 @@
 
 """Application level list creation."""
 
+import re
 import shutil
 import logging
 
 from contextlib import suppress
 from mailman.config import config
-from mailman.interfaces.address import IEmailValidator
+from mailman.interfaces.address import (
+    IEmailValidator, InvalidEmailAddressError)
 from mailman.interfaces.domain import (
     BadDomainSpecificationError, IDomainManager)
 from mailman.interfaces.listmanager import IListManager
@@ -35,6 +37,12 @@ from zope.component import getUtility
 
 
 log = logging.getLogger('mailman.error')
+# These are the only characters allowed in list names.
+_listname_chars = re.compile('[-0-9a-z_.]', re.IGNORECASE)
+
+
+class InvalidListNameError(InvalidEmailAddressError):
+    """List name is invalid."""
 
 
 @public
@@ -57,6 +65,8 @@ def create_list(fqdn_listname, owners=None, style_name=None):
         `fqdn_listname` does not exist.
     :raises ListAlreadyExistsError: when the mailing list already exists.
     :raises InvalidEmailAddressError: when the fqdn email address is invalid.
+    :raises InvalidListNameError: when the fqdn email address is valid but the
+        listname contains disallowed characters.
     """
     if owners is None:
         owners = []
@@ -64,6 +74,10 @@ def create_list(fqdn_listname, owners=None, style_name=None):
     # posting address.  Let these percolate up.
     getUtility(IEmailValidator).validate(fqdn_listname)
     listname, domain = fqdn_listname.split('@', 1)
+    # We need to be fussier than just validating the posting address.  Various
+    # legal local-part characters will cause problems in list names.
+    if len(_listname_chars.sub('', listname)) > 0:
+        raise InvalidListNameError(listname)
     if domain not in getUtility(IDomainManager):
         raise BadDomainSpecificationError(domain)
     mlist = getUtility(IListManager).create(fqdn_listname)
diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py
index f504e3dfd..db36d0b64 100644
--- a/src/mailman/app/tests/test_lifecycle.py
+++ b/src/mailman/app/tests/test_lifecycle.py
@@ -21,7 +21,8 @@ import os
 import shutil
 import unittest
 
-from mailman.app.lifecycle import create_list, remove_list
+from mailman.app.lifecycle import (
+    InvalidListNameError, create_list, remove_list)
 from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.domain import BadDomainSpecificationError
 from mailman.interfaces.listmanager import IListManager
@@ -39,6 +40,12 @@ class TestLifecycle(unittest.TestCase):
         self.assertRaises(InvalidEmailAddressError,
                           create_list, 'bogus address')
 
+    def test_listname_validation(self):
+        # Creating a mailing list with invalid characters in the listname
+        # raises an exception.
+        self.assertRaises(InvalidListNameError,
+                          create_list, 'my/list@example.com')
+
     def test_unregistered_domain(self):
         # Creating a list with an unregistered domain raises an exception.
         self.assertRaises(BadDomainSpecificationError,
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index d2d4a39d2..14d995190 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -109,6 +109,7 @@ Bugs
  * Fix ``mailman stop`` not stopping some runners due to PEP 475 interaction.
    (Closes: #255)
  * Update documentation links for ``config.cfg`` settings.  (Closes: #306)
+ * Disallow problematic characters in listnames.  (Closes: #311)
 
 Configuration
 -------------
-- 
cgit v1.2.3-70-g09d2


From 0ac6d8273cfb5fb60321867e108a3c6f7cee7bfb Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Sun, 12 Feb 2017 22:35:10 -0800
Subject: Made the allowable list name characters configurable.

---
 src/mailman/app/lifecycle.py            | 16 +++++++++++++++-
 src/mailman/app/tests/test_lifecycle.py | 23 +++++++++++++++++++++++
 src/mailman/config/schema.cfg           |  5 +++++
 src/mailman/docs/NEWS.rst               |  2 ++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py
index cfa01531b..6970de187 100644
--- a/src/mailman/app/lifecycle.py
+++ b/src/mailman/app/lifecycle.py
@@ -38,7 +38,7 @@ from zope.component import getUtility
 
 log = logging.getLogger('mailman.error')
 # These are the only characters allowed in list names.
-_listname_chars = re.compile('[-0-9a-z_.]', re.IGNORECASE)
+_listname_chars = re.compile('[-_.+=!$*{}~0-9a-z]', re.IGNORECASE)
 
 
 class InvalidListNameError(InvalidEmailAddressError):
@@ -76,8 +76,22 @@ def create_list(fqdn_listname, owners=None, style_name=None):
     listname, domain = fqdn_listname.split('@', 1)
     # We need to be fussier than just validating the posting address.  Various
     # legal local-part characters will cause problems in list names.
+    # First we check our maximally allowed set.
     if len(_listname_chars.sub('', listname)) > 0:
         raise InvalidListNameError(listname)
+    # Then if another set is configured, check that.
+    if config.mailman.listname_chars:
+        try:
+            cre = re.compile(config.mailman.listname_chars, re.IGNORECASE)
+        except re.error as error:
+            log.error(
+                'Bad config.mailman.listname_chars setting: %s: %s',
+                config.mailman.listname_chars,
+                getattr(error, 'msg', str(error))
+                )
+        else:
+            if len(cre.sub('', listname)) > 0:
+                raise InvalidListNameError(listname)
     if domain not in getUtility(IDomainManager):
         raise BadDomainSpecificationError(domain)
     mlist = getUtility(IListManager).create(fqdn_listname)
diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py
index db36d0b64..ec7d2b771 100644
--- a/src/mailman/app/tests/test_lifecycle.py
+++ b/src/mailman/app/tests/test_lifecycle.py
@@ -26,6 +26,7 @@ from mailman.app.lifecycle import (
 from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.domain import BadDomainSpecificationError
 from mailman.interfaces.listmanager import IListManager
+from mailman.testing.helpers import LogFileMark, configuration
 from mailman.testing.layers import ConfigLayer
 from zope.component import getUtility
 
@@ -46,6 +47,28 @@ class TestLifecycle(unittest.TestCase):
         self.assertRaises(InvalidListNameError,
                           create_list, 'my/list@example.com')
 
+    @configuration('mailman', listname_chars='[a-z0-9-+\]')
+    def test_bad_config_listname_chars(self):
+        mark = LogFileMark('mailman.error')
+        # This list create should succeed but log an error
+        mlist = create_list('test@example.com')
+        # Check the error log.
+        self.assertEqual(
+            mark.readline()[-83:-1],
+            'Bad config.mailman.listname_chars setting: '
+            '[a-z0-9-+\]: '
+            'unterminated character set'
+            )
+        # Remove the list.
+        remove_list(mlist)
+
+    @configuration('mailman', listname_chars='[a-z]')
+    def test_listname_with_minimal_listname_chars(self):
+        # This only allows letters in the listname.  A listname with digits
+        # Raises an exception.
+        self.assertRaises(InvalidListNameError,
+                          create_list, 'list1@example.com')
+
     def test_unregistered_domain(self):
         # Creating a list with an unregistered domain raises an exception.
         self.assertRaises(BadDomainSpecificationError,
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index 6b6362ab4..9386caed1 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -75,6 +75,11 @@ filtered_messages_are_preservable: no
 # The command should print the converted text to stdout.
 html_to_plain_text_command: /usr/bin/lynx -dump $filename
 
+# Specify what characters are allowed in list names.  Characters outside of
+# the class [-_.+=!$*{}~0-9a-z] matched case insensitively are never allowed,
+# but this specifies a subset as the only allowable characters.
+listname_chars: [-_.0-9a-z]
+
 
 [shell]
 # `mailman shell` (also `withlist`) gives you an interactive prompt that you
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 14d995190..ffd6c16a6 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -120,6 +120,8 @@ Configuration
    rules is not yet exposed through the REST API.  Given by Aurélien Bompard.
  * The default languages from Mailman 2.1 have been ported over.  Given by
    Aurélien Bompard.
+ * There is now a configuration setting to limit the characters that can be
+   used in list names.
 
 Command line
 ------------
-- 
cgit v1.2.3-70-g09d2


From 8f414b7e9b95e4148359b35109daf0fb16d2d933 Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Sun, 12 Feb 2017 23:10:47 -0800
Subject: Fixed broken tests.

---
 src/mailman/rest/docs/systemconf.rst      | 1 +
 src/mailman/rest/tests/test_systemconf.py | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/mailman/rest/docs/systemconf.rst b/src/mailman/rest/docs/systemconf.rst
index e61dd2cc9..213929a42 100644
--- a/src/mailman/rest/docs/systemconf.rst
+++ b/src/mailman/rest/docs/systemconf.rst
@@ -20,6 +20,7 @@ You can also get all the values for a particular section, such as the
     html_to_plain_text_command: /usr/bin/lynx -dump $filename
     http_etag: ...
     layout: testing
+    listname_chars: [-_.0-9a-z]
     noreply_address: noreply
     pending_request_life: 3d
     post_hook:
diff --git a/src/mailman/rest/tests/test_systemconf.py b/src/mailman/rest/tests/test_systemconf.py
index 570342dea..afa788719 100644
--- a/src/mailman/rest/tests/test_systemconf.py
+++ b/src/mailman/rest/tests/test_systemconf.py
@@ -43,6 +43,7 @@ class TestSystemConfiguration(unittest.TestCase):
             filtered_messages_are_preservable='no',
             html_to_plain_text_command='/usr/bin/lynx -dump $filename',
             layout='testing',
+            listname_chars='[-_.0-9a-z]',
             noreply_address='noreply',
             pending_request_life='3d',
             post_hook='',
-- 
cgit v1.2.3-70-g09d2


From 1bde5f4b0f91831b22ebba135dc361ad9d622b3e Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Mon, 13 Feb 2017 00:20:18 -0800
Subject: Replaced assertEqual with assertRegex to account for difference in
 error messages between python versions.

---
 src/mailman/app/tests/test_lifecycle.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py
index ec7d2b771..3730291da 100644
--- a/src/mailman/app/tests/test_lifecycle.py
+++ b/src/mailman/app/tests/test_lifecycle.py
@@ -53,11 +53,12 @@ class TestLifecycle(unittest.TestCase):
         # This list create should succeed but log an error
         mlist = create_list('test@example.com')
         # Check the error log.
-        self.assertEqual(
-            mark.readline()[-83:-1],
-            'Bad config.mailman.listname_chars setting: '
-            '[a-z0-9-+\]: '
-            'unterminated character set'
+        self.assertRegex(
+            mark.readline()[-93:-1],
+            '^.*Bad config\.mailman\.listname_chars setting: '
+            '\[a-z0-9-\+\\\]: '
+            '(unterminated character set|'
+            'unexpected end of regular expression)$'
             )
         # Remove the list.
         remove_list(mlist)
-- 
cgit v1.2.3-70-g09d2


From c39ef107c1765f1eb937bfcaae2e23dda0b4c581 Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Sun, 19 Feb 2017 17:37:05 -0800
Subject: Updated rest/lists.py to handle list name error exceptions and added
 tests for for the exceptions.

Enhanced the InvalidListNameError exception to return the invalid name.
---
 src/mailman/app/lifecycle.py            |  4 ++++
 src/mailman/app/tests/test_lifecycle.py |  2 +-
 src/mailman/rest/lists.py               | 10 +++++++++-
 src/mailman/rest/tests/test_lists.py    | 22 ++++++++++++++++++++++
 4 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py
index 6970de187..e373bf4d9 100644
--- a/src/mailman/app/lifecycle.py
+++ b/src/mailman/app/lifecycle.py
@@ -44,6 +44,10 @@ _listname_chars = re.compile('[-_.+=!$*{}~0-9a-z]', re.IGNORECASE)
 class InvalidListNameError(InvalidEmailAddressError):
     """List name is invalid."""
 
+    def __init__(self, listname):
+        super().__init__('{}@any.example.com'.format(listname))
+        self.listname = listname
+
 
 @public
 def create_list(fqdn_listname, owners=None, style_name=None):
diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py
index 3730291da..4b198d973 100644
--- a/src/mailman/app/tests/test_lifecycle.py
+++ b/src/mailman/app/tests/test_lifecycle.py
@@ -54,7 +54,7 @@ class TestLifecycle(unittest.TestCase):
         mlist = create_list('test@example.com')
         # Check the error log.
         self.assertRegex(
-            mark.readline()[-93:-1],
+            mark.readline(),
             '^.*Bad config\.mailman\.listname_chars setting: '
             '\[a-z0-9-\+\\\]: '
             '(unterminated character set|'
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index e665515ec..4b467afb3 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -20,8 +20,10 @@
 from lazr.config import as_boolean
 from mailman.app.digests import (
     bump_digest_number_and_volume, maybe_send_digest_now)
-from mailman.app.lifecycle import create_list, remove_list
+from mailman.app.lifecycle import (
+    InvalidListNameError, create_list, remove_list)
 from mailman.config import config
+from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.domain import BadDomainSpecificationError
 from mailman.interfaces.listmanager import (
     IListManager, ListAlreadyExistsError)
@@ -246,6 +248,12 @@ class AllLists(_ListBase):
         except BadDomainSpecificationError as error:
             reason = 'Domain does not exist: {}'.format(error.domain)
             bad_request(response, reason.encode('utf-8'))
+        except InvalidListNameError as error:
+            reason = 'Invalid list name: {}'.format(error.listname)
+            bad_request(response, reason.encode('utf-8'))
+        except InvalidEmailAddressError as error:
+            reason = 'Invalid list posting address: {}'.format(error.email)
+            bad_request(response, reason.encode('utf-8'))
         else:
             location = self.api.path_to('lists/{0}'.format(mlist.list_id))
             created(response, location)
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index 7bb7bd333..75a81f3cb 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -129,6 +129,28 @@ class TestLists(unittest.TestCase):
         self.assertEqual(cm.exception.reason,
                          'Domain does not exist: no-domain.example.org')
 
+    def test_cannot_create_list_with_invalid_posting_address(self):
+        # You cannot create a mailing list which would have an invalid list
+        # posting address.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists', {
+                     'fqdn_listname': '@example.com',
+                     })
+        self.assertEqual(cm.exception.code, 400)
+        self.assertEqual(cm.exception.reason,
+                         'Invalid list posting address: @example.com')
+
+    def test_cannot_create_list_with_invalid_name(self):
+        # You cannot create a mailing list which would have an invalid list
+        # posting address.
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists', {
+                     'fqdn_listname': 'a/list@example.com',
+                     })
+        self.assertEqual(cm.exception.code, 400)
+        self.assertEqual(cm.exception.reason,
+                         'Invalid list name: a/list')
+
     def test_cannot_create_duplicate_list(self):
         # You cannot create a list that already exists.
         call_api('http://localhost:9001/3.0/lists', {
-- 
cgit v1.2.3-70-g09d2


From 5d2883cb22e36bc8fe1275ae911b2fd97e6e9da1 Mon Sep 17 00:00:00 2001
From: Mark Sapiro
Date: Mon, 20 Feb 2017 18:36:36 -0800
Subject: Moved InvalidListNameError class definition to
 mailman.interfaces.mailinglist. Added more documentation on
 config.mailman.listname_chars. Made a couple of minor tweaks.

---
 src/mailman/app/lifecycle.py            | 15 ++++-----------
 src/mailman/app/tests/test_lifecycle.py |  4 ++--
 src/mailman/config/schema.cfg           |  4 +++-
 src/mailman/interfaces/mailinglist.py   | 10 ++++++++++
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py
index e373bf4d9..e79073929 100644
--- a/src/mailman/app/lifecycle.py
+++ b/src/mailman/app/lifecycle.py
@@ -23,11 +23,11 @@ import logging
 
 from contextlib import suppress
 from mailman.config import config
-from mailman.interfaces.address import (
-    IEmailValidator, InvalidEmailAddressError)
+from mailman.interfaces.address import IEmailValidator
 from mailman.interfaces.domain import (
     BadDomainSpecificationError, IDomainManager)
 from mailman.interfaces.listmanager import IListManager
+from mailman.interfaces.mailinglist import InvalidListNameError
 from mailman.interfaces.member import MemberRole
 from mailman.interfaces.styles import IStyleManager
 from mailman.interfaces.usermanager import IUserManager
@@ -37,18 +37,11 @@ from zope.component import getUtility
 
 
 log = logging.getLogger('mailman.error')
-# These are the only characters allowed in list names.
+# These are the only characters allowed in list names.  A more restrictive
+# class can be specified in config.mailman.listname_chars.
 _listname_chars = re.compile('[-_.+=!$*{}~0-9a-z]', re.IGNORECASE)
 
 
-class InvalidListNameError(InvalidEmailAddressError):
-    """List name is invalid."""
-
-    def __init__(self, listname):
-        super().__init__('{}@any.example.com'.format(listname))
-        self.listname = listname
-
-
 @public
 def create_list(fqdn_listname, owners=None, style_name=None):
     """Create the named list and apply styles.
diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py
index 4b198d973..dd320c1da 100644
--- a/src/mailman/app/tests/test_lifecycle.py
+++ b/src/mailman/app/tests/test_lifecycle.py
@@ -60,8 +60,8 @@ class TestLifecycle(unittest.TestCase):
             '(unterminated character set|'
             'unexpected end of regular expression)$'
             )
-        # Remove the list.
-        remove_list(mlist)
+        # Check that the list was actually created.
+        self.assertIs(os.path.isdir(mlist.data_path), True)
 
     @configuration('mailman', listname_chars='[a-z]')
     def test_listname_with_minimal_listname_chars(self):
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index 9386caed1..fde431b60 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -77,7 +77,9 @@ html_to_plain_text_command: /usr/bin/lynx -dump $filename
 
 # Specify what characters are allowed in list names.  Characters outside of
 # the class [-_.+=!$*{}~0-9a-z] matched case insensitively are never allowed,
-# but this specifies a subset as the only allowable characters.
+# but this specifies a subset as the only allowable characters.  This must be
+# a valid character class regexp or the effect on list creation is
+# unpredictable.
 listname_chars: [-_.0-9a-z]
 
 
diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py
index ddc5a13b6..f066ba3c8 100644
--- a/src/mailman/interfaces/mailinglist.py
+++ b/src/mailman/interfaces/mailinglist.py
@@ -18,11 +18,21 @@
 """Interface for a mailing list."""
 
 from enum import Enum
+from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.member import MemberRole
 from public import public
 from zope.interface import Attribute, Interface
 
 
+@public
+class InvalidListNameError(InvalidEmailAddressError):
+    """List name is invalid."""
+
+    def __init__(self, listname):
+        super().__init__('{}@'.format(listname))
+        self.listname = listname
+
+
 @public
 class DMARCMitigateAction(Enum):
     # Mitigations to apply to messages From: domains publishing an applicable
-- 
cgit v1.2.3-70-g09d2