HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Security vulnerability in version 2.0.18.8
It appears 2.0.18.8, which I understand is the stable release, has one unpatched published security vulnerability and possibly another one, as documented at:
http://www.exploit-db.com/exploits/25720/
To verify, I logged in on my forum as an admin, created a new discussion and saved a draft. I then opened another browser, signed as an ordinary user, and was able to see the mentioned draft by using a simple .../index.php?p=/post/editdiscussion/0/1
I did not try the other flagging XSS vulnerability described in the same page
4
Comments
I can confirm the Flag plugin vulnerability does exist. This to me is worse than someone accessing a draft I have written. I will see if can post an update fix....unless someone beats me to it
okay, here is the fix for the flag vunerability. what you do is open up class.flagging.plugin.php in the flag plugin. On line 253
$Comment = $Sender->Form->GetValue('Plugin.Flagging.Reason');
add right below on line 254
add:
[redacted]
Maybe someone wants to take a stab at the draft issue? I can confirm it is also an issue.
UPDATE: Correct fix is here: https://github.com/vanillaforums/Garden/commit/e117af7c126944514749b2e01244392077d0ac5c
I disabled the flagging plugin in my installs, Just no need and the check mark did not go away after disabling I had to hack something to make it go away....
about the drafts , I am using forceredirect plugin and it won't allow anyone to get to that area, they must be logged in to do so. And only to the user can see their drafts ... so no I do not have that issue with drafts.
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
@vrijvlinder, I had no choice with flagging, I have issues with human spammers promoting their business.
As for the drafts can you share more about how you are using force-redirect? Are you just using it as normal. Any mods?
there are only a limited amount of controllers allowed and draftscontroller is not one of them.
only the controllers you allow will be open for view to guests. when you use this plugin.
http://vanillaforums.org/addon/forceredirect-plugin
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
@vrijvlinder, awesome plugin, works like a charm.
If you use the flagging plugin, just change that one line and you are good to go. So now all vulnerabilities mentioned are closed by this thread
cheers @Lolo999 for pointing them out
You should add to it mobilefriendly true so it also works on mobile. I noticed it may not have it ? I need to check.
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
@vrijvlinder looks like it does work on mobile without the flag
ok cool then , I just don't remember if I added it or not, I know it works for me too. Thanks for verifying
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
my pleasure
I appreciate you creating such a helpful add-on to harden security.
Thanks ! A user with similar issues inspired it, but
well, I did not create it all alone, @peregrine helped me fix the errors I made and @hgtonight came up with the array idea to add or remove controllers as needed. So it was a small community effort.
It is very simple but it does the job. The only thing I would like it if there was a way to specify plugin controllers. Does not seem to work if you use say : extrapageplugincontroller or plugincontrollerextrapage, I don't think there is a way to do that is there ?
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
I'm not entirely sure about what forceredirect does. From what I read in this thread I understand it restricts contents so guests cannot view them.
If that is the case, the vulnerability is half closed, as registered users could still access the drafts of other registered users (and possibly modify them?), as I described in my example above.
I tried accessing the drafts of user and it is not possible. Not using
http://www.mysite.com/forum/drafts to access someone else's drafts
and not with this either, the link to the draft
/forum/discussion/1/0#Form_Comment
The link to the drafts of other users does not appear to the admin or other users.
The way this works is it check for user id to get the correct page of drafts.
If you could possible give a structured link to be able to verify this it would be great
If this was for real, I think more people would come out and say something like "my drafts have been erased by someone else" That would suck for sure. I can't verify it because I can't reproduce the potential issue
It restricts pages from guests and redirects to login.
❌ ✊ ♥. ¸. ••. ¸♥¸. ••. ¸♥ ✊ ❌
I don't want rain on anyone's parade, but this is not a viable solution at all, seems to be lifted from some unrelated question I answered. Please be careful.
If you want to be secure you should use Gdn_Format::Text() instead.
As it is already being use in one place on output then it should be applied on all the reports.
This is really @Tim 's responsibility, since he is the author.
grep is your friend.
I've just confirmed the Draft vulnerability. You can also delete another person's draft (discussion or comment) using a request like:
YOURSITE/vanilla/drafts/delete/DRAFTID/TRANSIENTKEY?Target=drafts
In class.draftscontroller.php, after
// Delete the draft $Draft = $this->DraftModel->GetID($DraftID);You can add the following:
if ($Draft->InsertUserID !== $Session->UserID) { Gdn::Dispatcher()->Dispatch('DefaultPermission'); exit(); }and in class.postcontroller.php, after
public function EditDiscussion($DiscussionID = '', $DraftID = '') { if ($DraftID != '') { $this->Draft = $this->DraftModel->GetID($DraftID);You can add:
if ($this->Draft->InsertUserID !== Gdn::Session()->UserID) { Gdn::Dispatcher()->Dispatch('DefaultPermission'); exit(); }That should work as a hotfix for the issue before the Vanilla team can release a new version.
No rain on my parade. While @Tim may be an author of a plugin, I tried to help to close the vulnerability made public in our community on a Sunday. Isn't that the point of community, to help out others? My only error was I should have credited you @x00 for your answer somewhere else, which I had read earlier and which inspired. I was trying to offer a patch as fast as I could to a vulnerability made public in this forum. Sometimes you have to do that before some can release the perfect patch. I will try to use your solution instead, but I am not sorry I offered a solution...
The thread where @x00 the earlier solution came from was : http://vanillaforums.org/discussion/comment/186558 (for full transparency--I tracked it down)
the patch that I have worked out from his advice (above) is on line 254:
$Sender->SetData('Plugin.Flagging.Reason', Gdn_Format::Text($Comment)); //per @x00this will ensure all flag inputs are text to your database. I works for me. Please let me know if you know better with the proper code solution
My point that it was not a solution. I applaud you for trying to help, however, if people believed that would close the vulnerability, they might take it in good faith and not check back.
It was actually an answer to a specific unrelated question. All it does it remove urls, except in image tags. It is not a substitute for sanitation.
grep is your friend.
This is an adequate solution, until an official patch, but doesn't help if the someone already used this exploit, and that data iInow in the database, which is one of the reason why sanitation is done on ouput.
grep is your friend.