Dependency graph bypasses all ticket security

[Wichert Akkerman]

The dependency graph view of a ticket does not do any permission checks. This is a security problem on private trac sites since it creates a channel through which sensitive information about tickets (existence, dependencies and ticket titles) is revealed.

[Ryan J Ollos]

mitar has posted a ​patch. Closing ​ticket on GitHub as a duplicate.

mastertickets/web_ui.py

@@ -131 +131 @@
         return req.path_info.startswith('/depgraph')
 
     def process_request(self, req):
+       req.perm.require('TICKET_VIEW')
         path_info = req.path_info[10:]
 
         if not path_info:

[Mitar]

Ha. Nice one. I completely missed this one. :-)

[Mitar]

Hm, the links above are bad. I am not sure if this was my patch. I am also not sure if it addresses the thing correctly? It still just limits based on access to current ticket, not to dependencies. If I have access to current ticket but not to the dependency, I can still see the dependency in the graph, no?

[Ryan J Ollos]

The GitHub repository is private now and development has been moved back to trac-hacks. It looks like the patch wasn't posted by you though, it was posted by ​tinus-github.

I think you are right, we need to check permissions of each dependency before deciding whether to include it in the graph (or at least, whether to include any information about it, such as the summary).

[anonymous]

And you need to check if a user has TICKET_VIEW before you allow them to see anything at all...

[Ryan J Ollos]

Replying to anonymous:

And you need to check if a user has TICKET_VIEW before you allow them to see anything at all...

You mean the patch from comment:1? It is a good first step, but it doesn't take care for TracFineGrainedPermissions.

[Ryan J Ollos]

In 17718:

TracMasterTickets 4.0.4dev: Require TICKET_VIEW

Refs #9944.

[Ryan J Ollos]

Leaving ticket open for adding TracFineGrainedPermissions checks.