[Fixed] Guessing searchid could show Admins' searches

[Jet]

I noticed a serious potential security problem when having many private forums in a board.

Let me try to explain: I'm an Administrator and for instance would make a search for all my posts (with the 'show results as posts' option, the one with 5-6 rows of the message body).
This way, a new search is inserted into db and all threads are looked for as I have top grants.

If anybody else with a lot of time would try all searchid on the url line within search.php?s=&action=showresults&searchid=xxxxx, most probably could find AND READ all my private posts everywhere.

Well, I think this' a small bug I simply solved comparing within showresults section
- $search[userid] <-> $bbuserinfo[userid]
and if not equal, the performer gets a show_nopermission()

But this simple simple security hole made me though to many other potential ones.
Does anybody have some other idea about possible security bugs in other phps ?

Thanks

[Scott MacVicar]

Its not exactly a serious security hole but yes it will allow them to read the first 30 characters of your post which could be a bit off.

There was another security issue with search.php that allowed you to read the first post of a thread that was being moderated.

To fix this problem

below
$search=verifyid("search",$searchid,1,1);
add
if($search['userid'] != $bbuserinfo['userid']) {
eval("standardredirect(\"".gettemplate("redirect_search")."\",\"search.php?s=$session[sessionhash]\");");
exit;
}

This way you can just humor them by showing the search in progress page then they end up back at search.php

Moving to bugs

[Jet]

Originally posted by PPN
Its not exactly a serious security hole but yes it will allow them to read the first 30 characters of your post which could be a bit off.

Thanks a lot for the move and the other fix.
Actually, with a lot of private and hidden forums, even the thread title within the hidden forum shown title is a big problem to me... :(
(I mean the documented HERE (http://www.vbulletin.com/forum/showthread.php?s=&postid=241966#post241966) online.php bug about the thread title in inherited-forum permissions... )
Is there any known fix or workaround waiting for vb3 ?

I'm looking for other potential hole points among all scripts: any idea where to search ?

:p

Thanks again

[Scott MacVicar]

The permissions problem is sorted in vB3.

There is no real way to find these bugs in the scripts apart from testing, in your case you were doing url manipulation in order to find a hole.

[Jet]

I had to manage a lot with vb permissions and their strange behavior with cache or inheritance (for hacks), so I guess it's a big problem and it needs a complete re-structuration from scratch.

Only, asking about something to search for within scripts; I fear something like '[title]' .... And check any &action against...
:( :( :(

Thanks again.

[Paul]

Originally posted by PPN
add
if($search['userid'] != $bbuserinfo['userid']) {
eval("standardredirect(\"".gettemplate("redirect_search")."\",\"search.php?s=$session[sessionhash]\");");
exit;
}

This way you can just humor them by showing the search in progress page then they end up back at search.php

Wouldn't it be better to show_nopermission();?


if($search['userid'] != $bbuserinfo['userid']) {
show_nopermission();
}


Seeing the first 30 characters of a post in a forum you don't have access to could be a major issue for certain web sites, depending on the content in those forums.

Paul

P.S. -- Jakeman is looking for someone who can speak french in the Moderators Lounge.

[Chen]

Originally posted by LoveShack
P.S. -- Jakeman is looking for someone who can speak french in the Moderators Lounge.
Oh no, the world has come to an end. :)

[Scott MacVicar]

Why show a no permission?

They have access to the search function, so we might as well redirect them to the first part of it.

[Paul]

Originally posted by PPN
Why show a no permission?

They have access to the search function, so we might as well redirect them to the first part of it.

For the same reason we show no permission when someone tries to edit another users's post. They might have editing capabilities on their own post, but we don't take them to one of theirs.

It seems that when a user changes the search id (or perhaps clicks on a pasted link that contains one), they are actually accessing something that is using credentials other than their own. I suppose the real problem here is that the search id somehow ignores the credentials of the user accessing the search page and uses the credentials of the original user who performed the search.

The ideal solution, in my opinion, would be to tailor the results of any search to the credentials of the user accessing the specific search id. That way any danger of someone posting a search url (which I have done myself in the past when trying to show another user something, not knowing about this vulnerability) is nil.

Paul

[Tommy Boy]

If you're applying PPN's or LoveShack's fix, you should also make the following modification. Otherwise, there might be situations where people can't reuse their own searches.

Look for:if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE showposts='".intval($showposts)."' AND query='".addslashes($wheresql)."' AND postids='".addslashes($goodpostlist)."' AND querystring='".addslashes($query)."'")) {
Replace with:if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE showposts='".intval($showposts)."' AND query='".addslashes($wheresql)."' AND postids='".addslashes($goodpostlist)."' AND querystring='".addslashes($query)."' AND userid='$bbuserinfo[userid]'")) {
Look for:if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE query='".addslashes($wheresql)."'")) {
Replace with: if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE query='".addslashes($wheresql)."' AND userid=$bbuserinfo[userid]")) {Repeat the last replacement as many times as needed (5 in my board).

[Scott MacVicar]

There is no code which would affect that part of the file, the check is simply done when they attempt to load the finished results.

Which is after the simplesearch action, and it redirects to back before this stage.

[Mike Sullivan]

No, Tommy Boy is correct. The actual fix just basically removes the code that he edited because it was essentially useless anyway.

[ccd1]

Hmm, let's see, so when I post something in a private forum, it shows up as my last post on my public profile--this will be taken care of in VB3?

[Paul]

Originally posted by baragon0
Hmm, let's see, so when I post something in a private forum, it shows up as my last post on my public profile--this will be taken care of in VB3?

In version 2.2.6, that is not the case. Log out and look at your profile after posting in a private forum.

Paul

[ccd1]

I dun it, and I'm running 2.2.6

[Steve Machol]

Have you hacked your forums? The default vB does not show the private message posting if the person viewing it does not have permission to view that forum. It will show the most recent public posting instead. I have just tested and verified this with my unhacked 2.2.6 forum.

[ccd1]

Maybe I'm just being stupid--oh well.

Expected Response:

Maybe?

[Freddie Bingham]

Originally posted by baragon0
Maybe I'm just being stupid--oh well.

Expected Response:

Maybe? Please give us a link to your profile where this private post is being shown.

[scotty]

THX @ tommy boy!!

user of my board reportet a similiar problem after installing the fix:

the link "Search for all posts by this user" gives an empty (resp. a redirected) result of the search in that case, that another user has searched the posts for this user not long ago.

att: the error occoures only for normal users - with my admin account the results where fine!

so you have to look for the second code snippet from tommy boy twice!!
(the second is in the section "Start posts by user x"


edit: fixed typo

[Tommy Boy]

Maybe I wasn't clear about it the first time, so I updated my post to make sure it's clear. You have to replace all occurrences of the search string (5 times in my board).

BTW: This problem happened to me on my board as Administrator. User groups have nothing to do with it, it can happen to anybody.

What's THY? :)

[scotty]

Originally posted by Tommy Boy
Maybe I wasn't clear about it the first time, so I updated my post to make sure it's clear. You have to replace all occurrences of the search string (5 times in my board).
oh - ok - I corrected only the first and the last - thanks!

What's THY? :)
öhm - :rolleyes: - that should be "THX" for "thanks"...

I corrected it...

[mishkan]

Thank you to all who have found and addressed this problem. :) However, I'm just not smart enough to put it all together. :rolleyes: Could someone please :D post another message to consolidate and summarize the fix? Step-by-step instructions would be great. Thanking you in advance! :D :D :D

[Paul]

Before making this or any other alterations to the forum code, ensure that you completely backup your database and your files.

In search.php:

Find:


$search=verifyid("search",$searchid,1,1);


BELOW, insert:


if($search['userid'] != $bbuserinfo['userid']) {
show_nopermission();
}


Find all instances of:

if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE showposts='".intval($showposts)."' AND query='".addslashes($wheresql)."' AND postids='".addslashes($goodpostlist)."' AND querystring='".addslashes($query)."'")) {


REPLACE each instance with:


if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE showposts='".intval($showposts)."' AND query='".addslashes($wheresql)."' AND postids='".addslashes($goodpostlist)."' AND querystring='".addslashes($query)."' AND userid='$bbuserinfo[userid]'")) {


Find all instances of:

if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE query='".addslashes($wheresql)."'")) {


REPLACE each instance with:

if ($getsearch=$DB_site->query_first("SELECT searchid FROM search WHERE query='".addslashes($wheresql)."' AND userid=$bbuserinfo[userid]")) {


Save and upload search.php

[Joshua Clinard]

Wasn't there also some code that should be removed because it is useless?

[Tommy Boy]

As Ed said, you can either make these replacements, or remove all the SQL checks. Both work, but only the replacements are described in this thread.