Fix security-problem: If user hasn't permission on a message (notably

files and content properties) and is on the nosy list, the content was
sent via email. We now check that user has permission on the message
content and files properties. Also add a regression test for this.

git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4393 57a73879-2fb5-44c3-a270-3262357dd7e2

diff --git a/CHANGES.txt b/CHANGES.txt
index ecd1235256b6d6fb6ce10479b9988bf83d203883..ba91730e705af059a815a23a163583ce2cfdf78b 100644 (file)
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -16,6 +16,11 @@ Fixes:
   for reporting.
 - Fix some format errors in italian translation file
 - Some bugs issue classifiers were causing database lookup errors
+- Fix security-problem: If user hasn't permission on a message (notably
+  files and content properties) and is on the nosy list, the content was
+  sent via email. We now check that user has permission on the message
+  content and files properties. Thanks to Intevation for funding this
+  fix.
 
 
 2009-10-09 1.4.10 (r4374)

diff --git a/roundup/roundupdb.py b/roundup/roundupdb.py
index 998b29051912d85425cde44b47bf019abbf7ed12..570c767e49e0bd7bc6a00ac18dfb98876f68cce8 100644 (file)
--- a/roundup/roundupdb.py
+++ b/roundup/roundupdb.py
@@ -227,18 +227,29 @@ class IssueClass:
             seen_message[recipient] = 1
 
         def add_recipient(userid, to):
-            # make sure they have an address
+            """ make sure they have an address """
             address = self.db.user.get(userid, 'address')
             if address:
                 to.append(address)
                 recipients.append(userid)
 
         def good_recipient(userid):
-            # Make sure we don't send mail to either the anonymous
-            # user or a user who has already seen the message.
+            """ Make sure we don't send mail to either the anonymous
+                user or a user who has already seen the message.
+                Also check permissions on the message if not a system
+                message: A user must have view permisson on content and
+                files to be on the receiver list. We do *not* check the 
+                author etc. for now.
+            """
+            allowed = True
+            if msgid:
+                for prop in 'content', 'files':
+                    if prop in self.db.msg.properties:
+                        allowed = allowed and self.db.security.hasPermission(
+                            'View', userid, 'msg', prop, msgid)
             return (userid and
                     (self.db.user.get(userid, 'username') != 'anonymous') and
-                    not seen_message.has_key(userid))
+                    allowed and not seen_message.has_key(userid))
 
         # possibly send the message to the author, as long as they aren't
         # anonymous

diff --git a/test/test_mailgw.py b/test/test_mailgw.py
index 749a93c0472f493014d60cbaadcd8eab137cdebd..d763fb98701dccf40f699ff34b493a0d6bacb8aa 100644 (file)
--- a/test/test_mailgw.py
+++ b/test/test_mailgw.py
@@ -1893,6 +1893,55 @@ This is a second followup
         assert nodeid1 == nodeid2
         self.assertEqual(self.db.issue.get(nodeid2, 'title'), "Testing...")
 
+    def testSecurityMessagePermissionContent(self):
+        id = self.doNewIssue()
+        issue = self.db.issue.getnode (id)
+        self.db.security.addRole(name='Nomsg')
+        self.db.security.addPermissionToRole('Nomsg', 'Email Access')
+        for cl in 'issue', 'file', 'keyword':
+            for p in 'View', 'Edit', 'Create':
+                self.db.security.addPermissionToRole('Nomsg', p, cl)
+        self.db.user.set(self.mary_id, roles='Nomsg')
+        nodeid = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <chef@bork.bork.bork>
+To: issue_tracker@your.tracker.email.domain.example
+Message-Id: <dummy_test_message_id>
+Subject: [issue%(id)s] Testing... [nosy=+mary]
+
+Just a test reply
+'''%locals())
+        assert os.path.exists(SENDMAILDEBUG)
+        self.compareMessages(self._get_mail(),
+'''FROM: roundup-admin@your.tracker.email.domain.example
+TO: chef@bork.bork.bork, richard@test.test
+Content-Type: text/plain; charset="utf-8"
+Subject: [issue1] Testing...
+To: richard@test.test
+From: "Bork, Chef" <issue_tracker@your.tracker.email.domain.example>
+Reply-To: Roundup issue tracker <issue_tracker@your.tracker.email.domain.example>
+MIME-Version: 1.0
+Message-Id: <dummy_test_message_id>
+X-Roundup-Name: Roundup issue tracker
+X-Roundup-Loop: hello
+X-Roundup-Issue-Status: chatting
+Content-Transfer-Encoding: quoted-printable
+
+
+Bork, Chef <chef@bork.bork.bork> added the comment:
+
+Just a test reply
+
+----------
+nosy: +mary
+status: unread -> chatting
+
+_______________________________________________________________________
+Roundup issue tracker <issue_tracker@your.tracker.email.domain.example>
+<http://tracker.example/cgi-bin/roundup.cgi/bugs/issue1>
+_______________________________________________________________________
+''')
+
 
 def test_suite():
     suite = unittest.TestSuite()