SMF Development > Bug Reports

Recent Posts does not properly adhere to unapproved topics

(1/3) > >>

Arantor:
2.0.2 - happens on this very board. In fact I swear I reported this before, though I seem to recall at the time it was concluded to be caching - it definitely isn't.

If a topic is unapproved but a moderator replies, the moderator's post is automatically considered approved, even if the topic isn't, and so Recent Posts shows that moderator's reply (as m.approved = 1, as apparently no check for t.approved = 1 or user has can_approve)

(For those playing along at home, I'm 99% certain Wedge actually patched this when Nao added topic privacy, which is probably why I never thought too much about it until I noticed it here again just now)

mashby:
I love this part!

--- Quote ---For those playing along at home
--- End quote ---
I cannot say you're wrong regarding this bug report. I just don't see this board as being subject to approval posts. Do you mean another board like Site Showcase? Can we try to replicate it there or am I just being Jane, the ignorant slut?

Arantor:
By 'this board' I mean this site.

You won't be able to see it, because you have approval rights and thus aren't subject to the same lack of permissions I am.

Basically what happens is that when Old Fossil replied to a topic in Showcase saying that the site shouldn't be using a default theme, I was able to see the reply even though I wasn't able to see the topic. The topic is not yet approved, but *his post* is, so his post shows up in Recent Posts (action=recent) even though the topic isn't approved, just because the test is only applied on the post itself, not the topic as a whole.

While this wouldn't necessarily be a big deal on most forums, it could be.

Consider how often the suggestion is made to implement user/staff privacy, and to use post moderation and 'just not approve posts' - except that staff replies will be visible through action=recent. I'd possibly even go so far as to suggest this is a privacy issue.

mashby:
Your specifics are extremely enlightening. I do see what you are referring to. I took a look at the (!) thing that appears on Site Showcase and it sure does reflect your findings. I complete agree with your findings regarding privacy/security. Yes, it might be minor in impact, but the philosophy of it certainly isn't being adhered to in the site code. I, for one, have no idea how to fix it, but maybe it will be addressed in 2.1.1? :)

Arantor:
It's relatively low on this site, but knowing how others may choose to use post moderation, I can't rule out that it has a potentially nasty privacy consequence.

FWIW, I'd suggest it should be patched in the next 2.0.x branch, not a 2.1 fix.

That said, let me elaborate a bit more now I've had time to examine the code. The offender, this beast in Recent.php:


--- Code: --- // Find the 10 most recent messages they can *view*.
// !!!SLOW This query is really slow still, probably?
$request = $smcFunc['db_query']('', '
SELECT m.id_msg
FROM {db_prefix}messages AS m
INNER JOIN {db_prefix}boards AS b ON (b.id_board = m.id_board)
WHERE ' . $query_this_board . '
AND m.approved = {int:is_approved}
ORDER BY m.id_msg DESC
LIMIT {int:offset}, {int:limit}',
array_merge($query_parameters, array(
'is_approved' => 1,
'offset' => $_REQUEST['start'],
'limit' => 10,
))
);
--- End code ---

It confirms one supposition and disproves another: it is based solely on m.approved, but I suspected it would elect not to carry out that test if the user was a moderator - it does not, it simply filters solely on approval.

Now, the reason for that is pure performance: there's no join to the topics table, thus the state of the topic being approved or not approved is not known at that point, and doing that does potentially slow it down significantly.

The simplest rewrite to include this would merely be:


--- Code: --- // Find the 10 most recent messages they can *view*.
// !!!SLOW This query is really slow still, probably?
$request = $smcFunc['db_query']('', '
SELECT m.id_msg
FROM {db_prefix}messages AS m
INNER JOIN {db_prefix}boards AS b ON (b.id_board = m.id_board)
INNER JOIN {db_prefix}topics AS t ON (t.id_topic = m.id_topic)
WHERE ' . $query_this_board . '
AND m.approved = {int:is_approved}
AND t.approved = {int:is_approved}
ORDER BY m.id_msg DESC
LIMIT {int:offset}, {int:limit}',
array_merge($query_parameters, array(
'is_approved' => 1,
'offset' => $_REQUEST['start'],
'limit' => 10,
))
);
--- End code ---

This would confine it to approved posts only (and still in the boards the user can see) which would close the loophole, but would also remove the ability of moderators (who can approve) from seeing unapproved topics entirely.

Personally, I don't think that's a huge deal (considering there are other places where this is already an issue, e.g. in the unread posts where unapproved topics do not turn up IIRC even if you would otherwise have permission to see them)

Navigation

[0] Message Index

[#] Next page