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

CommentFileSizeAuthor
#15 comment_reply_access_6.patch1.87 KBkilles@www.drop.org
#12 comment_reply_access_5.patch1.65 KBchx
#11 comment_reply_access_4.patch1.65 KBchx
#10 comment_reply_access_3.patch1.61 KBchx
#7 comment_reply_access_2.patch1.56 KBchx
#6 comment_reply_access_1.patch1.56 KBchx
#4 comment_reply_access_0.patch1.51 KBchx
#3 comment_reply_access.patch1.36 KBchx

Comments

pyromanfo’s picture

pyromanfo commented

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.

moshe weitzman’s picture

moshe weitzman commented
Project: Taxonomy Access Control » Drupal core
Component: Code » comment.module
Priority: Normal » Critical

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.

chx’s picture

chx commented
Assigned: Unassigned » chx
StatusFileSize
new comment_reply_access.patch1.36 KB

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.

chx’s picture

chx commented
StatusFileSize
new comment_reply_access_0.patch1.51 KB
chx’s picture

chx commented
chx’s picture

chx commented
StatusFileSize
new comment_reply_access_1.patch1.56 KB
chx’s picture

chx commented
StatusFileSize
new comment_reply_access_2.patch1.56 KB
Anonymous’s picture

(not verified) commented

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

Anonymous’s picture

(not verified) commented

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

chx’s picture

chx commented
StatusFileSize
new comment_reply_access_3.patch1.61 KB

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...

chx’s picture

chx commented
StatusFileSize
new comment_reply_access_4.patch1.65 KB

I also forgot to check whether node_load was successful or not...

chx’s picture

chx commented
StatusFileSize
new comment_reply_access_5.patch1.65 KB
Anonymous’s picture

(not verified) commented

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

chx’s picture

chx commented

Dries commited.

killes@www.drop.org’s picture

killes@www.drop.org commented
StatusFileSize
new comment_reply_access_6.patch1.87 KB

Here's a 4.5 patch. Untested.

Anonymous’s picture

(not verified) commented