From: schlatterbeck Date: Mon, 30 Nov 2009 14:45:44 +0000 (+0000) Subject: Fix security-problem: If user hasn't permission on a message (notably X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=356fa5f58300aa8ec33c602e8944f35bef1238e3;p=roundup.git 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 ecd1235..ba91730 100644 --- 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 998b290..570c767 100644 --- 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 749a93c..d763fb9 100644 --- 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 +To: issue_tracker@your.tracker.email.domain.example +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" +Reply-To: Roundup issue tracker +MIME-Version: 1.0 +Message-Id: +X-Roundup-Name: Roundup issue tracker +X-Roundup-Loop: hello +X-Roundup-Issue-Status: chatting +Content-Transfer-Encoding: quoted-printable + + +Bork, Chef added the comment: + +Just a test reply + +---------- +nosy: +mary +status: unread -> chatting + +_______________________________________________________________________ +Roundup issue tracker + +_______________________________________________________________________ +''') + def test_suite(): suite = unittest.TestSuite()