Roundup Tracker - Issues

Issue 1442835

classification
"private" queries not fully private, fully editable
Type: security Severity: major
Components: Web interface Versions: 1.4
process
Status: closed fixed
:
: richard : ajaksu2, arno-, jpend, richard, rouilj
Priority: normal : patch

Created on 2006-03-04 01:00 by arno-, last changed 2009-03-12 03:04 by richard.

Files
File name Uploaded Description Edit Remove
actions_query.diff ajaksu2, 2009-02-25 22:37 Arbitrarily block mere Users from EditCSV'ing Queries
Messages
msg2214 Author: [hidden] (arno-) Date: 2006-03-04 01:00
When I log as a normal user I don't have the menu :
"Class List" on my sidebar, but if I enter 
http://localhost:8080/support/home?@template=classlist
directly on my url bar, I have access to the class list.
If can now go :
http://localhost:8080/support/query

By the way, I don't need to go first to
home?@template=classlist, if I go directly to query I
have access to the queries list.
I am supposed to be able to see only my own queries (or
the non privates ones), but I can see every queries.

I think this is due to the fact that the template
checks if I can edit the class, meaning creating an
item. And, as I can, it shows me csv of all the class.

This happens in roundup 1.1.1

A workaround I have thought of, is to check for the
permission on every item is HTMLClass.csv
(cgi/templating.py line 580)

for example, from line 587

for nodeid in self._klass.list():
    if self._db.security.hasPermission('View',
self._client.userid, self._classname, itemid=nodeid):
        l = []
        for name in props:
            value = self._klass.get(nodeid, name)
            if value is None:
                l.append('')
            elif isinstance(value, type([])):
                l.append(':'.join(map(str, value)))
            else:
                l.append(str(self._klass.get(nodeid,
name)))
            writer.writerow(l)
return s.getvalue()

another way I can think of would be to create
html/query.index.html and to use it instead of
html/_generic.index.html
msg2215 Author: [hidden] (anonymous) Date: 2006-03-04 01:45
Logged In: NO 

Actually, that's not necessarily the definition of "private" in this case - that is, 
there wasn't seen a need to actually formally protect the "private" queries with 
security declarations. It shouldn't be too hard to restrict visibility with the View 
permission and a check function.
msg2216 Author: [hidden] (rouilj) Date: 2006-03-06 04:32
Logged In: YES 
user_id=707416

Looks like Arno and I saw the same bug around the same time.

From the mailing list:

>Subject: Re: [Roundup-users] demo user able to edit all
>         queries in csv format (roundup 1.1.1)
>Date: Sun, 5 Mar 2006 09:17:00 +1100
>On Saturday 04 March 2006 13:29, John P. Rouillard wrote:
>> Can anybody confirm that the demo user is able to edit
queries using
>> the csv interface in the roundup-demo for release 1.1.1.
>
>Yes, all users can edit and view all queries. There's no
explicit security
>protection for editing "private" queries as "private" is
more of a
>convention in my mind. I guess in others' minds it means
more, so
>this should be considered a bug.
>     Richard
msg2217 Author: [hidden] (jpend) Date: 2007-09-20 23:41
I'm not convinced that this is really a bug. The classic template is just a template. I'm having a hard time seeing why the default should be to hide queries. What about the query is so private? Anyone can regenerate exactly the same query if they want to. If they word "private" is the sticking point then we could consider changing it to "personal" or something maybe more neutral sounding.

Users can certainly customize the templates if they don't like that default behaviour.

I'm willing to be convinced, though :)
msg2218 Author: [hidden] (richard) Date: 2007-09-21 02:09
I'm in favour of changing the wording to "personal".
msg3594 Author: [hidden] (ajaksu2) Date: 2009-02-25 22:37
IMO this is a serious hole that any useful tracker should fix ASAP.

It's not a problem with the 'private' wording, one User can steal
queries from other Users and even from Admin.

It's not a template issue either: you can mess with anyone else's
queries, something SearchAction blocks but EditCSVAction doesn't.

Pranksters can change the Query URL to something lighthearted, like
"@error_message=No Issues in this Tracker&@ok_message=Tracker Deleted
Successfully <br> <br> <br> <br> <br>"...

Attached patch is a stopgap fix, feedback welcome.
msg3618 Author: [hidden] (ajaksu2) Date: 2009-03-08 12:35
It turns out the problem is much worse than I thought, see issue 2550521.
msg3636 Author: [hidden] (richard) Date: 2009-03-12 03:04
Fixed in r4182
History
Date User Action Args
2009-03-12 03:04:40richardsetstatus: open -> closed
resolution: fixed
messages: + msg3636
2009-03-08 12:35:20ajaksu2setmessages: + msg3618
2009-02-25 22:37:04ajaksu2setfiles: + actions_query.diff
type: security
severity: normal -> major
title: "private" queries not fully private -> "private" queries not fully private, fully editable
keywords: + patch
nosy: + ajaksu2
versions: + 1.4
messages: + msg3594
2006-03-04 01:00:50arno-create