Bypass access via comments
nazadus - March 16, 2005 - 19:39
| Project: | Drupal |
| Component: | comment.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | chx |
| Status: | closed |
Description
I believe I have found a bug.
If you goto http://www.etherpunk.com/comment/reply/180 (possibly NSFW) it allows you to view the posting (while you don't have permission to actually post, it still allows the page to get displayed).
I found this out by using awstats on my box and found that a hidden page was getting hit fairly common that I really don't want getting shown (well, it's on the web, I know... but... I'd rather have more controlled access).
Does this belong in the comment section for not obeying TAC?
Can anyone confirm this on their site?
Kenny
#1
That's definitely something you need to take up with the comment module guys. It's not just taxonomy access control either, it's the core node_access hooks in Drupal. If they'll just check that before displaying a node for reply, that'd fix it no problem.
#2
filed under comment.module ... note that my big comment patch gets rid of this page entirely (consolidates under comment/edit) so it might make sense to apply my patch instead of fixing this.
#3
moshe , http://drupal.org/node/18656 this does not seem to affect the permissions of the comment/reply path.
I think the approach I have taken is blatantly simple: literally check for access.
#4
#5
#6
#7
#8
The discovery of this patch makes me wonder, whether we shouldn't centralize the access controll a bit more.
If a core module can show this kind of bug, contrib modules will almost certainly. I propose to do a node_access() check inside node_load.
Gerhard
#9
I also don't think that the patch is working.
node_access(arg(2)) should probably be node_access('view', $node) and the node needs loading before.
Gerhard
#10
OK, patch corrected.
I think adding node_access to node_load would break havoc 'cos there are some routines (even in core) which do not check whether the node_load was successful or not. This is a whole another topic, please make another issue.
What we need, IMHO is to patch this quickly...
#11
I also forgot to check whether node_load was successful or not...
#12
#13
The patch looks good now. I agree that centralizing the node_access checking should be another task. This patch is needed now and should also be backported to the 4.5 branch.
Gerhard
#14
Dries commited.
#15
Here's a 4.5 patch. Untested.
#16