See: http://drupal.org/node/1048906

The attached patch fixes the CSRF in og_forum:

OG Forum does not properly implement access controls on private forums it creates, which can lead to a private group's forums becoming public via Cross Site Request Forgeries (CSRF).

The following security issue is still open:

Additionally, OG Forum stores private group and forum information in a global vocabulary, which can lead to information such as group and forum names being disclosed to members not part of the private group.

Currently I have no idea how to fix that forum name have public taxonomy tags. I think there is only one solution: Revoke the feature of inserting those informations to the taxonomy system. Do you have other ideas?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pumpkinkid’s picture

http://drupal.org/project/private_taxonomy

Granted I haven't taken a look at the code, but what if we modified the module to make the vocabulary 'private' as it states in the module description?

Seeing that the code for D6 is dev and the maintainer has abandoned that project it may just be a start...

Going to check it out now...

crifi’s picture

I got a brainwave! I think for D6 we need to implement hook_term_path() and maybe hook_taxonomy(). For D7 the API is much better differentiated: http://api.drupal.org/api/drupal/modules--taxonomy--taxonomy.api.php/7

thePanz’s picture

I've ported your patch (looks good for fixing first issue) in my GIT og_forum repo (https://github.com/thePanz/og_forum/) adding few code cleanups (the management functions are now in a separated file og_forum.manage.inc)

Regards

nhoeller’s picture

Has anyone tried to port the CSRF patch to 6.x-2.2? When I installed 6.x-2.x-dev prior to implementing the patch, I found that users who were not part of any group were unable to view the list of public forums (issue http://drupal.org/node/1059090).

The first block of the patch at line 47 looks like it should apply as is on 6.x-2.2 while the second part at line 1275 matches code in 6.x-2.2 but at a different line number.

crifi’s picture

The best solution to fix the remaining security issue is to clean up the code of hook_db_rewrite_sql() and protect private forums for displaying in /taxonomy/%term for users not part of the group.

thePanz’s picture

@nhoeller: could you please try my GIT branch and confirm the issue about public forums?

@crifi: you're right: rewrite_sql could be the solution, instead I don't think that your first idea to go with hook_term_path() is the best one, in this latest case we should handle both forum_term_path() hook and our og_forum_term_path()... (IMHO)..

Regards

monotaga’s picture

@nhoeller: RE: #4, I'm using OG Forum 2.2 and have applied crifi's CSRF patch above without noticeable issue at this point. (Granted, the line numbers were slightly off for 2.2, but no issue.) As far as I can tell through simple testing, users not in any OGs are still able to see regular site forums (from the core Forum module) at /forum.

nhoeller’s picture

@nhoeller: could you please try my GIT branch and confirm the issue about public forums?

@thePanz, I definitely can try, but after looking through various documents about Drupal and GIT, I am not sure how I would go about it.

@nhoeller: RE: #4, I'm using OG Forum 2.2 and have applied crifi's CSRF patch above without noticeable issue at this point.

I successfully applied the patch as well. Forum visibility works as before, but moving forums around causes strange problems (forum title sometimes becomes Forums, only one forum topic displays in the list, sometimes forum disappears). I need to refresh my test environment, set up test cases and verify that the problem is not in the base OG Forum code.

crifi’s picture

nhoeller: I don't think that the described behavior could be a problem with my patch since I only changed closed functions. Please also empty the cache, because my patch has now update function which calls a menu rebuild.

thePanz’s picture

@nhoeller: to try my og_forum fork on GIT, visit this page https://github.com/thePanz/og_forum , "Download" it and replace the previous one.

@crifi: please drop me a line if you'd like to be added to my GIT repository members, we can work together on the same code..

Waiting for your comments! :)

Jim Kirkpatrick’s picture

+1

I also manually applied the CSRF patch in the OP to DEV without any side effects. Be good to commit that part of it soon so the main risk is removed.

AntiNSA’s picture

+1

pumpkinkid’s picture

I too have tested the patch and can verify that the CSRF problem seems to not be an issue anymore.

Any word on using rewrite_sql to take care of the second issue?

rkdesantos’s picture

I can also verify that the CSRF patch seems stable and working after a manual installation.

mgifford’s picture

I cloned the repository here - https://github.com/thePanz/og_forum/

seems to work alright in d6, but I need to do more testing.

PixelClever’s picture

It really doesn't seem to me that this is a security issue that merits the releases being shut down. Someone being able to view a private forum when they aren't supposed to doesn't really compromise the integrity or security of the overall site. It would make more sense to file this as a simple bug.

dlirit’s picture

Can someone summarize the current status - are both issues currently patched in the dev version?

Also:
1. how does one test the CSRF issue?
2. I'm using the latest (25th feb) dev pkg and seems like the private forums issue is fixed, is that the case indeed?

crifi’s picture

@Aaron Hawkins: This is still a security issue (kind of information disclosure, see CWE 200) since the issue opens information to the public which should be protected. This isn't the desired behavior. It's your decision to use the module with the patch above until there is a secure version again.

The current status is:
1. The CSRF is closed.
2. The information disclosure vulnerability is still open (to reproduce: mark forum as private, open the taxonomy term of the forum as non authenticated user).

And there's no commit or new package until the security team commits a patch and we have a new accepted maintainer for this project (Commit of 25th Feb was a GIT migration update).

dlirit’s picture

@crifi as un-authenticated user the user is always redirected to the login page no matter what and as another user not part of the group I tried several things but still I don't see any forum or forum topic being disclosed (and the forums are private)...
on the same note, I didn't see an option to add tags to a new forum topic I create...

thePanz’s picture

@dlirt: try using my GIT fork of og_module (link in previous posts).. a part of the security issue have been fixed there

crifi’s picture

@dilirt: I don't know, why it's so hard to comprehend the information disclosure. But I try to clarify what we are talking about:

0. Login as user UID1 or equivalent.
1. Enable OG Forum and dependent modules.
2. Create a (moderated/closed) group.
3. Go through the group menu to "Manage group forums" page. You see a forum "General discussion", mark this as private.
4. Insert a "secret topic" to the forum "General discussion".
5. Go to Administer -> Content management -> Taxonomy. Click to "list terms" of the vocabulary "forum" (OG forum creates this taxonomy vocabulary).
6. Copy the link to the taxonomy term of the groups "General discussion" forum into your clipboard.
7. Logout from drupal.
8. Call the URL of your clipboard. Ups!

pumpkinkid’s picture

So now that we agree that the CSRF issue is closed... Has anybody played with any way we could correct the issue that crifi provided the walk through for?

jvieille’s picture

sub

kenorb’s picture

sub

adam_b’s picture

sub

Julie Henn’s picture

sub

jen.c.harlan’s picture

subscribe

mgifford’s picture

Ok, so there was the initial patch about this issue - SA2011-004-og_forum-csrf.patch & then this code here:
https://github.com/thePanz/og_forum/

There are still nearly 2k sites using this module even though it has a security flaw.

I don't know if anyone has:

- Choose another, actively maintained module instead -- I don't know if there is one that does this function.
- Fix the module and then contact the security team to have your version reviewed and the project handed over to you following the abandoned project process -- Have @crifi or @thePanz sent the patches to the security team? Was there a response?
- Hire someone to fix the security bug so the module can be re-published (see this guide on how to hire a Drupal site developer) -- Anyone want to take over the ownership of this module http://drupal.org/node/251466 it's a big job, but if there are 2000 sites using it, there should be some way to pay to make it someone's priority.

I'll contact crifi & thePanz to see what they have done. Subscribing to this forum post isn't going to resolve the issue. It's a useful first step, but please take the next step and see if there is anything you or your organization can do to bring this back as a well maintained and secure module. There are lots of developers, including my company - http://openconcept.ca - who could make this a priority, especially if there is funding to help compensate for their time & experience.

It would be useful if there were steps documented to understand this problem better & also ways to test that the holes are successfully plugged. I did a simple install, but I need more details about the exploit than are provided here http://drupal.org/node/1048906 to know that the identified problems have indeed been resolved.

adam_b’s picture

Choose another module: AFAIK there's no other module which does this function - hence people are still using it.
Just in case people think it's unlikely to happen to them, I've had to install a CAPTCHA function to stop people exploiting this flaw and spamming a forum.

Fix the module: way beyond my capabilities I'm afraid. As you imply, I don't even know how to test it.

Hire someone to fix the security bug: I don't have the backing for this but would be glad to contribute if there's a fund somewhere.

mgifford’s picture

Ok, here's a page to contribute to this initiative $$:
http://ogforum.chipin.com/security-upgrade-for-og-forum

This isn't an estimate to get it done, but a ballpark. I just want to get the ball rolling on this. I don't even need to be involved in doing it.

pumpkinkid’s picture

@mgifford - Just because the security team suggest that we can pay a person or company to fix the issue doesn't mean that it is the best option. The fact is that while I can't speak for @crifi or @thePanz, (although I assume they have been looking at the issue as well) I have been trying to come up with a solution myself.

If you read our posts you will note that there are two issues that the security team brought to light. The first being the CSRF issue and the second being the information disclosure vulnerability as described on #18. While we have agreed that the CSRF issue has been corrected, we have attempted to discuss a solution for the vulnerability and have not reached a consensus of what that fix may be.

If you have even looked at the code you would see that the issue is that we are using a vocabulary term that is accessible to the public. That means that as stated in #21 even when a user should NOT have access to the group's private forums, you can circumvent the settings.

While we appreciate that you are trying to push for this module to be repaired, the fact of the matter is that setting up a chipin account and presenting your company as a solution seems more of a way to get paid to give us the solution... which seems to go against "Shinning a light on open source" and more like "Shinning a light on open source and then turning it off for non payment".

It seems to me that if you use this module, and your company has the resources to correct this issue, it would be in your best interest to fix it. In which case you may have already done so, and are simply asking us to pay to have it.

We have all been using this module with both security issues present way before it came to light that the module had these issues... If it really bothers you that the module shows as a security risk, than you can simply uninstall it!

crifi’s picture

For me it's not understandable why there are still people who are claiming that this issue is still undocumented. Please read the whole issue before wasting our time with those requests! And no, it's not my task as the developer to test my own patches or suggest anybody how to test it, because this is the task of a software tester. Please inform yourself about what's CSRF before testing the patch and analyze the code for the information disclosure vulnerability (to reproduce see #5 and #21).

For criticism at og_forum and alternative module solutions read the comments here: http://drupal.org/node/359889#comment-1204553

It makes no sense for me to send a half ready patch to the security team to incriminate them. I know the answer, because this patch "needs work" (see status) until the whole issue is fixed. Otherwise this project will be still abandoned and for me it makes no sense to have a secure module again, but it's still not maintained! Until now my hope is that for example pumpkinkid (who offers to maintain this module! Thanks!) or ThePanz will fix the remain of this issue and overtake the project.

I agree to pumpkinkid: It seems for me that you (@mgifford) want to have a actively maintained project, but you don't want invest in it or want to be paid for your work... Is this the matter of open source projects? Maybe I'm wrong, but your post has a negative connotation for me.
If you really have an interest to get this project back to life, It would be nice giving me something back and round up the patch.

pumpkinkid’s picture

Getting back on topic...

I was thinking based on what any of as have seen, do we know if there is any merit in making OG Forums dependent on TAC or TAC Lite?

Jim Kirkpatrick’s picture

@33 - Agreed...

It seems a good solution to make OG Forum dependent on another module that handles taxonomy access (TAC or maybe another lighter one) would be the best solution- no point reinventing the wheel or adding bloat...

pumpkinkid’s picture

@34 That's what I was thinking, I thought about handling it ourselves for our use case but it would be less of a maintenance hit I think...

crifi’s picture

The main problem I see in integrating other access control modules is that we have here a OG member based access control instead of a role based UAC like "taxonomy access control" and similar offers. Using those modules we must add the members to a role for access control. So your suggestion isn't the answer to everything since we shouldn't create a role for every forum we mark as private. Do you agree?

pumpkinkid’s picture

I agree, hadn't really thought about the roles piece.. I was thinking more about doing a taxonomy based access control where we would check if the user belonged to a group that had access to that vocabulary term... am I barking up the wrong tree?

mgifford’s picture

I want to see movement on this issue and see it regain it's status as a supported module on drupal.org. This thread had stalled and I'm happy to see that there is more discussion about ideas & approaches to bring this back.

I personally do contribute a lot to Drupal.org. I really can't also realistically volunteer any more time on D6 modules. Most of my efforts have been on D7 & now D8. However, I did want to call to the community of users who are subscribing here, waiting for a solution to address the security issues affecting their site. I'm just looking to eliminate on the the barriers that was stopping this from being a priority.

Having nearly 2000 sites be insecure for a month isn't good for the community. Being paid to do open source work isn't a conflict of interest. I was just bringing ideas from the security team forward into this issue queue.

pumpkinkid’s picture

@mgifford Understood, but the barriers you are talking about, (at least in my mind) are on how to solve the issue itself...

I find it unnecessary to continually come back to this thread to post "I still have no clue" or something the like. If you don't have the time to donate to the cause that is fine and I understand... but just the same way you don't have the time to donate, this is not my biggest priority for the reasons I stated earlier...

This module works as intended and flagged or not we have been using it with these issues all this time, so these 2,000 sites have not been insecure for a month... they have been "insecure" for a lot longer.

batje’s picture

subscribe

Anonymous’s picture

Assigned: Unassigned »

Hi Folks,

I will be spending some time next week working on the disclosure problem.

I'll contact Ryan today to see if he has been working on the problem ..

Best,
Paul Booker

llamatrix’s picture

Subscribe

ultimike’s picture

I spent some time this morning (along with liberatr) working on a fix for the taxonomy term pages issue (see http://drupal.org/node/1055424#comment-4153742 to reproduce). The fix is not complete - **it does not take into account all use cases** - it is meant to be a starting point for a potential fix. Specifically, it will not work for multiple terms passed in as "term1,term2,term3" (for example, taxonomy/term/1,2,3).

What we came up with is one possible path to a solution - shortly after we got things mostly working, we determined what might be an even better potential solution as well. First up, our solution...

We added some code to the og_forum_db_rewrite_sql() function for taxonomy/term/%termid pages to filter out nodes that the current user doesn't have access to because they're not a member of the group that contains the forum that the term belongs to. We wrote a og_forum_get_node_sql() function (very similar to the current og_forum_get_sql() function) to handle crafting the join and where clauses.

The new code works similar to the existing code in og_forum_db_rewrite_sql(), as it acts on a specific URL - namely "taxonomy/term/%termid". It is a bit tenuous though, because we're also searching the query string for the "tn" term_node table alias (we're actually searching for "tn" or "tn0", "tn1", etc...) When the conditions are met, the new og_forum_get_node_sql() function is called.

Another possible way of solving this issue if via an "access control" function for the taxonomy/term/%termid menu item. This would need to parse each term passed in and determine if the user is allowed to view the page. It would be an all or nothing sort of thing - if 3 termids are passed in and the user only has access to 1 of them, they would still get an access denied for the entire page.

Each time we thought we came close to a complete solution, the "term1,term2,term3" argument issue appears to make things **much** more complex.

I'm posting this incomplete solution here (although it appears to work fine for single terms passed in) in hopes that it will kickstart the development process for a comprehensive fix.

A patch is attached and is based on the latest version of the code from https://github.com/thePanz/og_forum/

-mike

ultimike’s picture

Rats - that was the wrong patch - it still has some debugging code in it - the correct one is attached here.

-mike

danielm’s picture

Subscribe

ultimike’s picture

In my previous comment 43 (http://drupal.org/node/1055424#comment-4251500), I indicated that we thought a better way of fixing this issue was to possibly use Drupal's node access control system. Combine that with PumpkinKid's idea of utilizing another module gave me the idea to look at the code of the Taxonomy Access Control Lite (TAC Lite) module. This module, "governs access to nodes based on the taxonomy terms applied to the nodes" - this sounds familiar to me.

Diving into the code a bit, I think this is something that should be explored. While I don't think we should make TAC Lite a dependency of OG Forums, we can certainly borrow some code from it and exploit it for this effort. In particular, the following 3 functions seem like they might be able to be leveraged for OG Forums:

As far as I can tell (after reviewing the TAC Lite code for about 15 minutes), I believe that "tac_lite_node_access_records()" creates node_access grants each time a node is saved based on the taxonomy terms applied to the node. In OG Forum, the taxonomy terms are the different forums. The "tac_lite_node_grants()" function provides the appropriate grants to a particular user. The tac_lite_db_rewrite_sql() function prevents queries from finding nodes that have terms that the user isn't allowed to view. To me, this seems like the a good portion of the work that is necessary to fix this security issue. These functions (and a few additional associated functions) will need to be re-worked for OG Forums, but it looks like it is doable.

I'd love for someone to take a look at the TAC Lite code for a second opinion.

-mike

pumpkinkid’s picture

@ultimike Funny.. I was just about ready to make a pretty in depth reply to you about that but the more I got into it the more I kept second guessing myself... I went back to this page after giving up and deciding to look into the code again before replying and there you were with exactly what I wanted to say! :-) Thanks!

That being said, I understand why you wouldn't want to make OG Forum dependent of TAC lite... But wouldn't that be best? In my case, I use TAC lite on my site, having essentially duplicate code would probably cause problems... where as if we leave it as a dependent we can take advantage of the work that gets put into TAC lite as well...

BTW, I was at your 7x7 New Things In Drupal 7 session at FL Drupalcamp! Nice running into you here :-)

ultimike’s picture

PumpkinKid,

The main reason I think we should rewrite the functions for OG Forum is because there are some TAC Lite-specific settings (which terms have which permissions for starters) that are used within them.

In OG Forums case, none of these will be necessary, as OG Forums already provides all the necessary information about which terms have which permissions. In this case, I think simpler is better.

We're not talking duplicate code, I'm talking about taking the logic used in TAC Lite and applying it to OG Forum. Then again, mine is only one opinion, I'd love to hear others.

Are you a Floridian? What's your real name?

-mike

pumpkinkid’s picture

@ultimike Alright I see your point... You mean take it and modify it as we need...

Yes I'm Floridian, But I hadn't gone to Drupal Camp before so I doubt you'd recognize me.

baff’s picture

Maybe I don't get the point, but if a forum topic content type is defined as OG type then I think there is no security problem ...

pumpkinkid’s picture

@baff If you follow the directions on #21 you'll see the problem...

baff’s picture

@pumpkinkid id did #21 - just to make sure - I tried e.g. node/1914/og/forum/8631

for me it is ok - but thats maybe because of other permission modules - no anonymous user is able to access a forum page - using content access, path access ....

For the meantime one could also redirect anonymous user by creating a rule ... or using taxonomy access or forum access?

ultimike’s picture

Hmmm - Forum Access (http://drupal.org/project/forum_access) is an interesting module that I hadn't considered before that might be helpful. Has anyone looked at its code to see if it would be helpful?

-mike

salvis’s picture

@ultimike: You asked me to come here. What can I help you?

@paulbooker: You assigned yourself to this issue. Are you working on it or not? It doesn't make sense to have multiple people working in parallel without coordination. This is a security-relevant module. If you cannot dedicate the required attention to keep it afloat, then please say so and step down.

It might make sense to create a sandbox project to work towards The Resurrection Patch. That way you could partition the problem into smaller pieces, commit partial solutions, provide snapshots so that everyone can help with testing (even those who can't handle patches), etc.

thePanz’s picture

I've created a sandbox for "OG Forum Resurrection" here: http://drupal.org/sandbox/thepanz/1110446 and I've added paulblocker to the admins.. The 2.x code is based on my latest github commits, feel free to join the OG Forum resurrection process :)

ultimike’s picture

@salvis: I was hoping that you'd be able to provide some guidance on how best to fix the OG Forum security issue. I think that your expertise with the Forum Access module can help us with OG Forums - as it performs similar task as the security issue we're currently having with OG Forum: keeping private posts private.

Forum Access allows users with certain roles access to particular forums. OG Forum needs a similar capability but based on OG membership, not roles.

Specifically, can you take a look at what we're trying to accomplish and let us know if following a similar path as Forum Access is a suitable path forward?

Thanks,
-mike

salvis’s picture

I don't have time to get into OG and thus it doesn't make sense for me to look into your source code, but I'll try to help you by answering your questions. Telling by #992570: sql errors when creating a topic (trivial issues not getting fixed for months, lack of basic SQL knowledge and problem solving skills), the og_forum community is not in good shape, which is a pretty serious problem for a security-related module!

Technically, it might make sense to merge this into Forum Access, but I'm not looking for more work. Still, we should probably look into what exactly would be needed in addition to what Forum Access provides, to make it do what you need for OG. Are OGs defined by a vocabulary, like forums? Are OG forums in the forums vocabulary, or in the groups vocabulary, or...? Is there zero or one forum per group? or more? How do you create a forum for a group? Are there other administrative functions? A group has organizers, right? Are they the ones that also administer the group's forum (edit/delete of nodes and comments)?

keeping private posts private.

That's too broad for a concrete question. Using the hooks that you mention in #46 is certainly the way to go. But hasn't this part already been fixed (whatever that means in the context of og_forum)?

So what is the problem here? That the taxonomy terms are exposed? Look at how FA is handling it. Specifically, look at how forum_access_db_rewrite_sql() handles the case 'tid'. This takes care of hiding the terms that are not supposed to be visible, including the taxonomy/term/1,2,3 case. Obviously, it assumes that access is governed by role IDs. What exactly defines membership in a group and where is it stored in the database?

Let me make this clear: You can show me snippets, but I won't look into your code at large, and you won't get me to say that "it's good now" because I just can't take that responsibility without being involved. og_forum should probably be rewritten from scratch. I don't know the specs, but I don't think there's that much to it...

pumpkinkid’s picture

I actually had thought about Forum Access as I use it myself and love the capabilities it offers... but as mentioned, FA does it by roles and I didn't think it would apply any different than TAC/TAC lite...

It seems to me that the FA solution would make the changes at the wrong level... wouldn't that be per forum node as opposed to per term?

salvis’s picture

FA controls access to forum topics as well as to forums, based on roles.

I'm still wondering how group membership is defined...

ultimike’s picture

@salvis: group membership is defined by a og_groups entry in the $user object.

-mike

salvis’s picture

Yes, but in the database? Node access works by altering SELECT queries to JOIN with the appropriate tables. Once you identify how group membership is defined, it should be pretty straightforward to switch from roles membership to groups membership.

One og_groups entry? A user can be in multiple groups, no?

You're not really answering the questions that I asked in #57 — I'm not sure what I'm doing here...

McGrimm’s picture

+1

ultimike’s picture

@salvis: To answer your second question first - I asked you to participate in this issue to provide guidance to us. I know how busy most Drupal developers and module maintainers are, and once I saw the similarities between OG Forums and Forum Access I was hoping that you'd be able to make sure we are proceeding down a reasonable path without wasting too much time. Your comments in comment 57 above are very helpful and allowed me to confirm that a similar approach could possibly work for OG Forums.

Regarding "how is group membership defined": OG defines group membership in the og_uid table. Each record contains a nid of a group along with a uid of a user (there are some additional, less essential fields as well). You are correct in stating that a single user can be in multiple groups.

In Forum Access, it appears that forum post access is determined by:

  • What terms does the forum post have?
  • Which of those terms have limited access based on user role?
  • What user roles does the user have?

In OG Forum, the end result should be something similar:

  • What terms does the forum post have?
  • Which of those terms have limited access based on the a user's group?
  • Which groups does the user belong to?

This is a very simplistic view of things, I know, but the similarities is what is driving my current train of thought. Like you said, it seems reasonable to assume that we can just swap out the roles stuff for the groups stuff. Do you foresee any issues with this plan?

Thanks for your time,
-mike

salvis’s picture

forum.module makes sure that a node has only one forum term, which is $node->tid. I'd guess that's the same for og_forum.module.

Apparently, {og_uid} works exactly like {users_roles}, so the chances are good that you can transfer at least the concepts, if not the code.

What about the group organizers? They seem to be similar to FA's moderators, which are handled by the ACL module. Giving them the right comment links is quite tricky...

nyl_auster’s picture

I don't get it. I followed instructions in comment #21 and going to taxonomy/term/xxx as an anonymous user, i can *not* see my "secret topic". I have to edit my topic and mark it as "public" in order to see it at taxonomy/term/xxx.

Am i missing something ?

Anonymous’s picture

Catching up ..

ultimike’s picture

Nyl,

Try it with a fresh install of Drupal 6 and the related modules. I was not able to reproduce it at first on an existing site (I have no idea why), but once I tried it on a fresh install it was apparent.

-mike

pumpkinkid’s picture

@Ultimike - Just a stab in the dark here... but do you think it might have to do with Path or PathAuto? I wonder if figuring that out would allow a simpler fix... (At least enough to remove the current security issue)

Anonymous’s picture

#21 It looks as though to get a *private* message to appear you have to forget to enable the "Organic groups access control" module.

Still catching up ..

Anonymous’s picture

Assigned: » Unassigned
ultimike’s picture

@pumpkinkid - I don't think it is an issue with path or pathauto. From what I've seen in the code, it is a basic architectural issue of the way the module is attempting to keep private posts private.

-mike

pumpkinkid’s picture

@ultimike, I just meant if you thought it could be why #21 wasn't working on a site already established... Or something like that...

Just thinking of modules most of us install that would cause you and Nyl to not see the issue the first time around.

ultimike’s picture

@pumpkinkid: Understood. I really have no idea why I couldn't reproduce it on the one existing site I tried it on. When it comes down to it, the fewer variables the better - understanding the simple case (clean install, as few as modules as possible) is sooooo much easier than trying to debug/fix an issue like this on an existing site.

I'm in the process of trying to figure out (with my client) which way they want to go with this issue.

-mike

progga’s picture

I was happily testing the patch. But then noticed that all my private forum nodes have suddenly become public !!!@#! Upon further scrutiny, it turned out that it has nothing to do with the patch. I did a node access permission rebuild (from "/admin/content/node-settings/rebuild") and that has got rid of all the existing node permissions from the node_access table in the database. I then updated a few of the supposedly private nodes and they became private again. Is anybody able to reproduce this behavior?

The problem seem to be the 'load' operation of the hook_nodeapi() implemenation. Unlike the "presave" operation, it does not add the "og_groups" property to the given node object. So when the permissions are rebuilt, the og_groups module simply ignores the forum posts. The following patch seem to have fixed the problem:

diff --git a/og_forum.module b/og_forum.module
index c0cff29..51aaa5a 100644
--- a/og_forum.module
+++ b/og_forum.module
@@ -473,6 +473,10 @@ function og_forum_nodeapi($node, $op, $teaser = NULL, $page = NULL) {
     case 'load':
       if ($node->type == 'forum' && $og_forum_nid = db_result(db_query('SELECT nid FROM {og_term} WHE
         $node->og_forum_nid = $og_forum_nid;
+
+        $gid = og_forum_gid_from_tid($node->tid);
+        $node->og_groups = array();
+        $node->og_groups[$gid] = $gid; 
       }
       break;
     case 'prepare':

But I am still wondering if this is some kind of a settings issue or is it really a bug.

jeffschuler’s picture

subscribing

codekarate’s picture

Subscribing. I need this for a website that has to launch within the next month and am willing to help out anyway I can. Anyone have an update as to where these patches are in the development/testing process. I am willing to dive into the code to help out, but I want to make sure I am not duplicating work that someone else may have already done.

If someone can suggest (or possibly post, if there has been an update) the most relevant patch, I will begin testing it.

pumpkinkid’s picture

@smthomas The module works "as is"... the only reason it was flagged was for a small issue that may not even be that much of a security threat depending on how the site is designed... This is not to say however that any help you would like to offer is not appreciated...

The reason I mention this is due to the fact that you have a launch deadline and personally, based on my personal experience of how similar offerings of help have been met, I strongly suggest you just use the module "as is" and if you are still interested come back and help out...

I'm not trying to be a jerk... I'm just a bit sour that I offered to not only help, but to take ownership of this module, and not only was I shoved off to the side (someone basically said they will fix it) repeat attempts to start the conversation have only fizzled away...

thePanz’s picture

@pumpkinkid: don't take it so bad :) I think that when you'll post your solution for this issue the request to be the a co-maintainer of this module will be accepted. I implemented some fixes and posted them into my OG Forum sandbox (http://drupal.org/sandbox/thepanz/1110446). I don't know how many are using it, the only thing I care is that I gave my 2cents. If you want I can add you as a contributor to my sandbox and then share out progress into fixing this issue.

BTW I find strange that only few people effectively try to solve this blocking issue respect to the great number of websites where OG Forum is used. Did they already solved the issue and forget to share the solution? or are they waiting for someone to fix it "for free"? ... again, my 2 cents..

Regards

pumpkinkid’s picture

@thePanz Just in case, I was not referring to you... The person I was referring to flat out said they will take ownership of the module and continue working on the fix... yet we have not heard anything back from him... that's what got me on a sour note...

I agree, I find it strange that not many are jumping to help solve this, but at the same time, I find that there are a few different attempts at solving this problem, and we have not, in any way shape or form, agreed on which method to take and move forward with...

We should really pull together our efforts and discuss what we are doing to fix this... notice that for the exception of @ultimike we've not truly discussed any roadblocks each of us are experiencing trying to solve this...

As for myself, I kinda stepped aside when I felt shoved off, but I'm getting ready to get back on this soon...

ultimike’s picture

@pumpkinkid - thanks for the mention in your previous post.

Just to give you guys a status update of my participation on this issue - I was hoping to be able to provide a fix for this issue by way of some client work. I have a client who was interested in this module, but in the end, what they wanted was a bit different (a blend of OG Forum and Premium modules - so that non-group members could see previews of private posts). So, over the past few weeks, that's where I've been spending my time.

I still believe that the future of OG Forum lies in a proper node access implementation and that we can learn a lot from the Forum Access module.

I'll assist as much as I can, but I don't have the bandwidth to drive the bus right now.

-mike

jean84’s picture

Did they already solved the issue and forget to share the solution?

i think its like security is invisible till you are hacked.
My homepage is still in alpha / beta so i am waiting. Also i have no idea of php, but i hope it will be fixed...

pumpkinkid’s picture

Alright, I hate to scratch a healing wound... but this thread isn't going anywhere... At this point, does anyone have an alternative solution to using this module?

I think we need to start discussing alternative ways of reaching the functionality OG Forums provided as at this rate, even if we do fix this, anyone upgrading to D7 will still be SOL... Especially considering that OG is supposed to be different re the way it functions in D7...

Any Ideas?

batje’s picture

About 2 years ago, i extended some code to an issue that tried to apply proper Drupal security to this module.

I ran into design issues that you would have to change the functionality of OG_Forum for, which at that point, i didnt feel like addressing. (Like, it is possible to create og_forum posts that do not have the of field set. There is no check on this and people might have posts that dont have this field set.)

So, only if some of these choices are made (which i think people are willing to do), this patch might actually work.

http://drupal.org/node/280355#comment-1600924

That might be a start that can push this thing forward.

pumpkinkid’s picture

Thank you!

I will take a look at this patch ASAP.

pumpkinkid’s picture

While testing, I had forgotten that this issue is not 100% reproduce-able as described on #21... This was brought up on #67 and I remember I had the same issue (I just had it again) where the results of the steps on #21 are "There are currently no posts in this category".

I'd like to bring up again, maybe we can just find out what causes this inconsistency and try to either make og_forum dependent on a module or make it so that it recommends that dependency... Who knows, maybe its just a setting....

pumpkinkid’s picture

Ok... I think I narrowed it down a bit...

I found that when a brand new site is enabled with only OG, OG Forum and its dependencies... access is granted to anonymous when following the steps in #21.

Enabling "Organic groups access control" the steps in #21 do not allow anonymous to access the term... however, if you followed the steps referenced and then enable Group access control, you will notice that following exactly the same steps again prevent you from seeing the new forum post, but still lets you see the one created before enabling group access control. Now, if you go to the post settings and rebuild the node access permissions, you will notice that the forum posts created AFTER enabling group access control show up for anonymous...

Looking at the database table for node_access, I found that initially (with group access control enabled) the table has two entries for each forum post...

NID      GID         REALM             GRANT_VIEW                GRANT_UPDATE               GRANT_DELETE
4          1        og_admin             1                          1                              1           
4          1      og_subscriber          1                          0                              0           

On Rebuilding the permissions, they get reset to just one entry

NID      GID         REALM              GRANT_VIEW               GRANT_UPDATE                GRANT_DELETE 
4          0           all                   1                          0                              0           

So... question is... is it really a OG_Forums issue or a OG access control issue?

salvis’s picture

Install Devel Node Access, enable its debug mode and the second DNA block to investigate this.

pumpkinkid’s picture

Not that I have any experience with the DNA Module, but I'm not sure how that identifies what the issue is...

That being said, I see that the module flags the node_access entries and states that there is a problem with them... The status states "alien" and that they "Should NOT be in {node_access} because of unknown origin!"

So I understand that there seems to be an issue with the way the module is handling the permissions and how the permissions would get rebuilt... so that puts me no farther than I was as I do not yet know how the node_access table works...

Any ideas?

pumpkinkid’s picture

Interesting side note... on saving the nodes after the permissions have been rebuilt, the correct permissions (the ones that deny anonymous viewing) re-appear....

batje’s picture

I read through my patch and discovered a typo

+ 'gr ant_view' => TRUE,

Note the space. That doesnt look good.

This patch introduced 3 new realms, that you should see: og_forum og_forum_public og_forum_author.

What is also does, is when the access is recalculated, it wipes *all* access rules for nodes that have any of the 3 realms attached. That would be all nodes created *after* the patch is applied.

I dont have the software installed anymore, so am just reading code.

pumpkinkid’s picture

I was trying to manually apply the patch and I couldn't find any of the original lines to replace... how long ago was it written? I'm not sure if my version was the latest one, but I did see it was meant to add new realms.

salvis’s picture

If DNA says "alien", that means that the grants are not returned by node_access_acquire_grants(), which is most likely caused by og_forum failing to properly implement hook_node_access_records().

batje’s picture

The patch adds a lot of code, and only inserts 2 lines.

hook_node_access_records() is not implemented. Sorry, i dont remember that caused issues. AFAIK i could see the realms with devel at the time. (yes, 2 years ago).

salvis’s picture

Every node access module needs to implement hook_node_access_records() to provide its records.

Are you inserting rows directly into the {node_access} table? That obviously doesn't work.

batje’s picture

sorry, i was sleeping, in the patch is a clear og_forum_node_access_records

pumpkinkid’s picture

Just to be clear... I am testing my original version of this module... I have not yet patched it with the one provided above.

mxmilkiib’s picture

subscribe

pumpkinkid’s picture

I haven't had a chance to keep pursuing this further... does anyone have any experience correcting issues similar to that found on #86?

baff’s picture

As OG Forum is essentia for my site I solved it for the meantime by using path access module and not allowing anonymous users to access forum paths and using content access not allowing anonymous users to see forum posts.

In addition I set forum topic content type as Standard group post (typically only author may edit).
on admin/content/node-type/forum so only group members can see those posts if group forum is set to private.

Scyther’s picture

subscribe

davidgibbens’s picture

Thought people might be interested to know that over on drupal commons someone has developed an alternative module to og_forums.
See this post http://commons.acquia.com/wiki/using-forums-drupal-commons
I have posted asking the module developer what they think about using it as a replacement for og_forum (ie how specific to drupal commons it is) and am awaiting a reply. But as groups in drupal commons rely on og it is difficult to see how this would not work.
The module itself is downloadable from https://github.com/that0n3guy/sts_commons_forum

pumpkinkid’s picture

Appreciate the heads up, however, based on a few things listed there, I am not able to use that version... Particularly these parts:

Doesn't really work very well when trying to use it with normal (non-group) forums.  It works best if all forums are associated with a group.
Has not been tested with using a forum post in multiple groups (multiple audiences).

I too am curious to see what they say are the advantages of using their version over OG Forum, other than of course, the outstanding security issue...

Let us know if you hear back from the maintainer.

jvieille’s picture

I don't see well the difference with OG Forum
Also, this module is not developed within the Drupal community, so it does not fit for me unless the code is being handled more openly.

shineshadow’s picture

subscribe

jcicolani’s picture

I hate being one of these, but... subscribe
Sounds like you're homing in on a solution. I'm a coding neophite and a Drupal intermediate beginner, but will help where I can. This is core to a site framework I'm going to use to launch several sites. So, yeah... I'll do what I can.

jvieille’s picture

Just trying to maintain the momentum: OG Forum is really a needed feature,

The CRSF being addressed, I understand the additional concern about

Additionally, OG Forum stores private group and forum information in a global vocabulary, which can lead to information such as group and forum names being disclosed to members not part of the private group.

but this should not by itself exclude this module from the Drupal repository - a simple, clear warning on the module page should make it, at least for some time.

The security team could be asked what they think of this as a temporary fix that would allow this module to survive. My fear is that if it remains banned for too long, the many existing sites will be obliged to find other certainly not satisfactory solutions, or to totally drop the functionality.

pumpkinkid’s picture

I completely agree with the sentiment...

mxmilkiib’s picture

The module is not fully excluded from drupal.org. From the project page; View all releases, Repository viewer (just the master head is empty).

pumpkinkid’s picture

So does anyone see a reason why we shouldn't just comit the fix to the module and continue solving this secondary issue moving forward?

pumpkinkid’s picture

I have just moved my initial request for ownership to the Drupal.org Webmasters issue queue...

http://drupal.org/node/1050008

Hopefully this will get the ball rolling on this.

dlirit’s picture

about the security disclosure vulnerability - even though forums might be marked as 'private' they still appear when taxonomy tags are directly accessed even for anonymous users but the actual content for these 'private' forums do not appear in the taxonomy listing. is that right ?

about og forum private/public functionality. marking a forum as being public seems to only make it so that the forum listing is showing up although it doesnt really render the forum and its post as public. meaning, if i make posts to the 'public' forum i need to explicitly set their audience to 'public' too for them to show up. is this correct?

im using thePanz's github

jvieille’s picture

Yes, this is roughly correct.
OG forum terms can be target like this http://mywebsite.tld/forum/128
- if the user is member of the forum's group, he gets to the forum normally, seeing the posts.
- If not, he reaches the forum page but gets a discouraging "No posts found." He also has an "Add discussion" button, and can actually attempt to create one, but he has no possibility to select a forum, so he will eventually fail to create this discussion.
- Anonymous users do not see anything at all
I don't understand the focus on this concern, as other modules do not behave better.
for example, OG vocab also does allow group terms to be reached from outside the group, while the tagged terms remain hidden (unless the content is public or also attached to on of his groups). Anonymous users can reach the term (the url is of the form http://mywebsite.tld/og/term/102/14)

I would not call that a security issue, just a nice to have usability improvement.

dlirit’s picture

thanks for the quick response jvieille though it's partial to what I was mentioning.
regarding the security concerns - so at the most part only the vocabulary name and terms are exposed but the fact that the terms themselves are exposed do not expose the actual forum topic nodes, yes?

please also comment on the public/private usability thing. I think it makes sense that if a forum is marked as private then posts to it can never be set to public, doesn't matter what the user is toggling on or off. the same if it's set to public - forum posts should be forced to be public (og_public = 1) while this is not the case because a forum post, being submitted to a group will be a group's post and then not public really... can anyone confirm this?

pumpkinkid’s picture

Fact is that the terms are accessible as anonymous and when the node access tables are set with the initial permissions, the nodes (forum posts) DO NOT appear on the term page... however... If you rebuild your permissions, this changes and it does in fact allow an anonymous user to view the nodes with that term... that is the issue I have highlighted above on #86.

As for the question dlirit has about the public/private settings... I'm not entirely sure I understand the question, but The way I see it working is that if you want, your group is "public" in the sense that the group is publicly listed in the group list and anyone can join your group with no need to be invited or added to the group manually... On the other hand a "private" group only allows for invited or manually added users to join the group...

The other side to dlirit's question is about the forum post's publicity... The public/private settings on the forum level have to do the same as with other OG nodes which allow a non-member of a group to view a particular node. While granted that a private group would not show as an available option to a regular user, the forum posts would be visible to them if you had a link that took them to that location on the site. This way a forum could be publicly accessible but maintained by a group.

Either way, I have already laid out the fact that the issue lies in the way the node access table is receiving it's data... for whatever reason the permissions are correct when they are first written, but when the permissions are rebuilt, the settings on the modules are not matching up to the original permissions.

salvis’s picture

#237634: Rename node_access_write_grants() to _node_access_write_grants() and discourage its use (i.e. calling node_access_write_grants() instead of node_access_acquire_grants()) is probably the reason for the permissions not surviving the permissions rebuild.

pumpkinkid’s picture

Thanks salvis! I'll look into that as soon as I can!

jvieille’s picture

I totally missed this permission rebuild issue.

Just to be clear about private / public :

At the group level: (with OG access and node not toggled "public")
- private means group is not listed, the group page is only accessible to its members
- public is the other way around. However, visibility of group posts remains bounded to group members only, and subscription to the group is totally under control, allowing free subscription as well as admin only control.
Posts in OG forum behave correctly as any posts in the group, though apparently the permission rebuild breaks this

At the post level
- setting a node "public" makes it visible regardless OG context (if you allow node visibility for anonymous, they can see it)

pumpkinkid’s picture

@jvieille Thanks! Your explanation is way more clearer than mine... That's what I get when I try to answer while doing having a real life conversation! :-P

I believe the key is in the rebuild issue... hopefully I get to take some time to look at the issue @salvis pointed out.

lirantal’s picture

Hey,

Joining in to the discussion I'd like to relate to a few things.
To begin I'll say that I'm basing these observations on Drupal Commons 1.6 and thePanz repo.

1. I think that some of the security issue is related to how OG, OG access and the OG forum behaves in terms of configuration. In my setup (explained in point 2), I don't have any issue with rebuilding permissions of nodes as @pumpkinkid pointed out.

2. Relating to the definition of Public Forums and Private Forums of OG - you're replies about the Organic Group mode (open/moderated/invite only/closed) are not relevant and are not the issue here. I am talking about the forum posts themselves to the forum and their visibility setting.
OG Forum has a button on 'manage forums' saying 'make public' or 'make private'. What does it do? Let's take a scenario where you want to make a 'General discussion' forum public - if you do that then what the 'make public' actually did is simply to make the forum view on /node/NID/og/forum listing the forums (and btw, it's listing BOTH public forums and private forums). Once the public forum is clicked it lists all of it's posts (well, public posts). But what happens when the user now creates a new forum topic on a the public 'General discussion' forum? since the post is a group type post then when it's saved it won't be a public post viewable to anyone but a group post. That's where I think the behavior of OG Forum is lacking. This also concerns the 'make private' option. If you do that to a forum and the user posts to this forum a new post and TOGGLES ON the 'Public' option in the Audience Checkbox (which is by default showing up to all users) then the post is made public and even though it exists within the 'Private Forum' forum it will still be accessible.

I've worked out a patch that checks whether a forum is marked as either private or public and overrides this Audience Checkbox 'Public' option and enforce setting it to be either private or public, depending on whether it was posted to a public or private forum which OG Forum controls with it's 'make private' or 'make public' buttons.

The above described behavior seems to me logical and the most intuitive. Even more, by default OG Access configuration sets the option of 'Visibility of posts:' to 'Visibility chosen by author/editor using a checkbox on the posting form. Checkbox defaults to public' - but why? this is exactly the issue where the user can toggle on this 'Public' checkbox in the 'Audience Checkbox' area in the post new/edit page and post a public post to a private forum.

With the above setting and my patch to OG Forum (based on thePanz repo) to enforce the public/private posts I do not have ANY issues with rebuilding permissions!

3. Regarding the security issue with terms of private groups being exposed I'm also seeking a solution for this at the moment.

This is the process to establish my configuration:

1. Install/Enable the 'forum' module
2. Go to Administer -> Content management -> Content types -> edit Forum topic -> set in Organic Groups to "Standard group post (typically only author may edit)."
3. Administer -> Organic Groups -> Organic Groups Configuration -> Group Details -> Audience checkboxes is toggled off.
4. Administer -> Organic Groups -> Organic Groups Access Configuration -> Visibility of posts option set to "Visible only within the targeted groups" and Private groups option set to "Group administrator chooses whether her group homepage and audience are private or not. Defaults to private"
5. Install/Enable the 'og_groum' module
6. Administer -> Organic Groups -> Organic Groups Forums -> everything in the "Forum publicity administration" area is toggled off. (possibly run 'Update old groups' if necessary).

If anyone is interested in checking out my patched og_forum and confirming this behavior please opt to do so.

pumpkinkid’s picture

@lirantal Thanks for taking the time to help try to figure this out... I was hoping someone would try to confirm my findings. That being said, I'm surprised that you did not have the same issue... by:
"With the above setting and my patch to OG Forum (based on thePanz repo) to enforce the public/private posts I do not have ANY issues with rebuilding permissions!"
do you mean it was after applying the patch that you did not have the rebuild issue, or did you not have the issue at all?

I'd like to see your patch so we can work in moving forward with getting this solved...

thePanz’s picture

If you want I can add you to my sandbox repo as co-maintainer: let's keep OG Forum alive! :)

lirantal’s picture

@pumpkinkid to be clear, the patch I'm talking about creates the behavior which I described, which is - if you set a forum to be public or private (with the 'make public' and 'make private' buttons) then OG Forum will enforce any post to the designated forum to be public or private, depending on that forum's settings. I believe that this is a coherent path to take.
Regarding the 'rebuilding permissions' thing - notice that one of my steps is to make the 'forum topic' content type to be standard group post in the content type settings (which isn't by default what happens on my drupal commons 1.6 install). Once that is set then yes, I can confirm that I'm not having any problems at all with the rebuilding permissions issue.
Ofcourse, if you have set a forum to be public, then made a post (which means that post is now set to be public), and then you make the forum private, then if you navigate to that former post then obviously it will be public and accessible. So, my patch enforces a post to be either public or private when it's posted to the forum, depending on the forum configuration but it doesn't retroactively checks all the posts that were made to the forum and changes their permissions if you move from public to private forum or the other way around.

@thePanz - Sure thing, please do. I forked your github, maybe add me as a developer there where I can apply these patches.

Regards,
Liran Tal.

lirantal’s picture

I believe I have fixed the security issues related with this SA.
Patch is provided against thePanz og_forum repository, please test and provide feedback.

Let's do another take on the security issue in question - install the og_forum plugin (preferably from thePanz github repo) and create 2 forums for a group - set one to be public and the other to be private.

Find out what are the term id's used for the public forum and the private forum you created and then verify the security issues are:
1. Go to http://your-drupal-site/taxonomy/term/
Result: the taxonomy term id will show the term name (the forum name) and it's description
2. Go to http://your-drupal-site/taxonomy/term/+
Result: the taxonomy term id will show the term name (the forum name) and it's description for both the private and public forums
3. Go to http://your-drupal-site/forum/
Result: the forums page will show up and it's listing *ALL* forums, both the Private and the Public forums that you created.
4. Go to http://your-drupal-site/forum/
Result: the forums page will show up, displaying the private forum listing page (though you should really NOT SEE the actual posts made to this private forum, it will be an empty listing)

With the patch provided applied, none of the steps will ever reveal the private forum name, description or posts and will either not return it as part of the results (like by accessing the group term id for listing all the forums) nor by accessing directly to the private forum.

Personal notice - while the provided patch will fix the security issues, I firmly believe that the behavior of the OG Forum module settings and the OG Access module settings are causing very odd scenarios of private/public semantics, where-as a user may create a post to a private forum but mark the post as 'public' in the OG Audience Checkbox (it's in the post settings page when you add/edit a post) there-fore resulting a public post appearing inside a private forum (that should never happen).
So on this note, I will make the necessary changes to this module on my github and make them available for everyone to use, hopefully this will be the 'next' OG Forum version.

thePanz’s picture

I've added Pumpkinkid and Lirantal to "OG Forum Resurrection" as co-maintainers.
Feel free to patch the source code as you like, following Drupal coding standards and KISS principle :)

Cheers

lirantal’s picture

You are welcome to try my version of the OG Forum module with:
1. CSRF patch (my repo was forked from thePanz repo so it contains his patching for this)
2. Security fixes discussed in this SA (originally commited on my repo)
3. Applying coherent public/private forum posts behavior

Repository: https://github.com/lirantal/og_forum

If you are upgrading, run the update.php script, I've specifically put an upgrade path in OG Forum so you can easily apply these behavior changes to the public/private forums thing.

I'd appreciate some feedback here and hopefully we have solved this module entirely and can bring it back to life on drupal's official projects directory.

Regards,
Liran Tal.

murkada’s picture

i go to test it lirantal :)

lirantal’s picture

@murkada waiting for your feedback...

lirantal’s picture

No feedback from anyone?
and to think everyone were crying out for these fixes for so long... :-)

I'm also looking at adding forums moderator support to OG Forum, which provides setting moderators at the forum level (i.e: different moderators for each forum (taxonomy term)).
Any thoughts? requirements? etc?

Regards,
Liran.

salvis’s picture

Concentrate on getting OG Forum back on track first.

Don't confuse the situation even further by talking about new features before the Security Team has given the green light, especially not in this thread!

lirantal’s picture

@salvis concentrate on what? I have already made it clear in several, very detailed, posts in the past 2 weeks that I have fixed the security issues related with this module and that they are available on my github as well as the patch I provided. Yet I did not receive any feedback.

I'm not going to wait around for a glimpse of feedback and I have much to do on this module, including the feature mentioned above.

Many here have been complaining about this module not getting attention and no one willing to really DO something about this (except very few here).
Well here it is, there is supposedly a fixed version now so how about we start getting some relevant feedback instead of this awful silence.

wcDogg’s picture

Didn't realize this could be fixed with a patch ...

http://drupal.org/node/694690

---------------------------------------------------------------------------------

I’m admittedly not good at anything other than following instructions and somewhat desperate for this module. Don’t know if I’m adding anything of value here, but I am getting some warnings …

I’m using Drupal 6.22 and OG 6.x-2.1.

Steps I followed:

  • Installed Forum using regular means
  • Didn’t understand the instruction “Trigger the forum vocabulary creation routine by visiting admin/forums” as admin/forums = page not found. Instead, visited admin/content/forum and created a test container with one forum, figuring this should cover it.
  • Installed og_forum using regular means
  • Checked Status report and was prompted to run the database update script. Update yielded the following warnings:

warning: array_merge() [function.array-merge]: Argument #2 is not an array in /home1/mysite/public_html/update.php on line 173.

warning: Invalid argument supplied for foreach() in /home1/mysite/public_html/update.php on line 337.

Curious know-nothing that I am, I continued by:

  • Peeked at the Forum Topic content type, noting that it was set to Standard Group Post
  • Visited admin/og/og_forums, left all of the defaults, clicked Update Old Groups, and saw a success message which I confirmed by looking at admin/content/forum
  • Visited a group node and checked out the Forums tab, which shows the General Discussion forum
  • Tried using the Create Forum Topic link and, not surprisingly, got the following:

warning: array_keys() [function.array-keys]: The first argument should be an array in /home1/mysite/public_html/sites/all/modules/og_forum/og_forum.module on line 291.

warning: in_array() [function.in-array]: Wrong datatype for second argument in /home1/mysite/public_html/sites/all/modules/og_forum/og_forum.module on line 291.

However, the new topic was created. Many, many thanks for your work on this mod.

- Lisa

salvis’s picture

@lirantal:

http://drupal.org/node/101497 gives some guidance.

It would definitely help if the various patch contributors could agree on a joint effort and a joint proposal to the Security Team. This will take patience and perseverance.

You could start by listing the issues, referring to relevant posts, explaining how you fixed the issues, comparing your fixes to the others that are floating around, explaining how you tested your fixes, convincing the other patch authors that your approach is the best one, etc...

Here is an example of a post that eventually lead to a core commit. The Security Team members are very busy people. They are not waiting for you. The onus is on you to provide the best possible presentation to make it worth their time to look into this.

salvis’s picture

@wcDogg/Lisa: Please don't go back and edit your old comments. We don't have time to figure out what you might have changed and where in your new comment that new gem might be hiding. Just post a new comment if you want to add something to the discussion.

If you must correct something then clearly mark your changes.

lirantal’s picture

Just to add to my comments regarding added functionality/features - another gap in terms of functionality is with deletion of forums. While the delete confirmation page says "Deleting a forum or container will delete all sub-forums and associated posts as well. This action cannot be undone.", in fact no posts are deleted for this forum but rather only the forum term table og_term is getting cleared from the forum term that's being deleted as you can see for yourself in og_forum_taxonomy():

    case 'delete':
      if (isset($obj['tid'])) {
        db_query("DELETE FROM {og_term} WHERE tid = %d", $obj['tid']);
      }
      break;

@salvis - I'm just as busy as the security team :-)

I'm willing to co-maintain this module with another developer - so whomever thinks he/she can ACTUALLY push this module forward in terms of communtiy relations with drupal's official team please raise your hand.

Regards,
Liran Tal.

thePanz’s picture

Good catch Liran, I set you as a co-maintaner of "OG Forum Resurrection" module, merge your edits with it, so we can have an *Active* development there.
Cheers

pumpkinkid’s picture

To All...

Been on Vacation so I've had no chance to test @lirantal's patch... I will start testing it as soon as I can.

That being said...

@lirantal So you're saying that then forum content type *must* be set as a standard group node in order for your patch to work? If so, then as I understand it, your patch is not fixing the problem... you are simply finding a workaround that may not work for everyone... Don't get me wrong, I haven't tested your findings yet... but I *NEED* to have forum posts work outside of the Organic Group functions as well as within them. Just trying to get clarification on this before going into testing.

jean84’s picture

There was a problem with taxonomy terms in private forums are visible to everyone.
I think maybe the taxonomy module has no feature to address this behavior so maybe to fix this one could make a new module which makes it possible to make taxonomy terms private.

salvis’s picture

#137: No. We've already talked about that. Forum Access does this nicely with the existing taxonomy.module, so OG Forum can do it, too.

pumpkinkid’s picture

#137 I still believe the issue lies in the whole broken node access on rebuild thing... if having the forum posts as standard group nodes corrects the issue as @lirantal pointed out, then the issue lies in figuring out the discrepancy between one and the other...

Note: I have yet to truly review the submitted patch.

salvis’s picture

Protecting terms is completely unrelated to node access (and thus to rebuilding permissions). Those are two separate tasks with no overlap.

pumpkinkid’s picture

@salvis isn't the problem in the security issue the fact that when you know the terms you can view the forum posts with that term attached to them? The rebuild issue causes the posts with a specific term to show up when they aren't supposed to...

Terms themselves are always visible to the user, but depending on the node access for each post, viewing a list of nodes with a specific term should still allow non private forum posts to show up...

I guess what I'm trying to get at is that you say there are two separate issues, but its the rebuild issue that allows the terms being visible to be an issue...

salvis’s picture

Hmm, I was thinking of this in the original post:

The following security issue is still open:

Additionally, OG Forum stores private group and forum information in a global vocabulary, which can lead to information such as group and forum names being disclosed to members not part of the private group.

But I'm sure you know better where exactly we stand.

pumpkinkid’s picture

Well, and that's what I was referring to as well... isn't the issue that due to the fact that the group and forum information is stored in a global vocabulary, it leads to the group and forum names being disclosed?

The issue is not that the vocabulary is public, but rather that because they are public, it leads to forum posts that aren't meant to be public to become visible...

At least... that's how I've been looking at it....

lirantal’s picture

Morning guys,

1. Regarding the exact security issue (mostly answering your question pumpkinkid) - when you know the terms, or even not as you can probably 'walk' through the vocabulary tree, you can access the terms and hence see the forum names which is exactly what the SA says ("group and forum names"). By no means is it possible to actually access the forum post UNLESS it was set to specifically to public (or the forum content type is not 'standard group post' which im not sure what results that yields if or if not you are doing the 'nodes permissions rebuild').

2. My patch addresses the issue of not allowing access to the terms (i.e: the group and forum names). I have actually done so much more on OG Forum than just that... I think someone is ought to test my entire work on the OG Forum as it includes more fixes and more improvements.

3.

@pumpkinkid - I think that setting forum content type to not being a standard group post might lead to unexpected behavior and it's beyond the scope of the module probably to handle that. It's called OG Forum for a reason, it was meant to handle forums in a group, just as blog and other contents are set to 'standard group post' (as can be seen in Drupal Commons dist - this setting is made for almost all content type on drupal).

pumpkinkid’s picture

@lirantal Thanks for the clarification. If your patch corrects the issue of displaying the forum names, then as you pointed out, the patch would be the solution if the behavior was the same between Standard Group Node and Not. The truth however is that it is not the case. As you stated, not having forum posts as a Standard Group Node causes problems, however, I do not think that it is outside of the scope of this module to support.

The fact is that modules should allow for new functionality, not hinder or disable core functionality unless absolutely necessary... I believe that we need to agree on whether or not the two use cases are going to be covered by this module or decide on if we need to force the forum posts to to be Standard Group Nodes and move forward.

Either way, I'm going to have to echo @salvis' concerns about trying to add new functionality within a patch that is strictly to correct an outstanding security issue.

jean84’s picture

"The issue is not that the vocabulary is public, but rather that because they are public, it leads to forum posts that aren't meant to be public to become visible..."

Yes so my suggestion would be to try to make the terms in the og forum vocabulary private altogether and only accessible for administrators who have the administer nodes permission.

pumpkinkid’s picture

@jean84 If it were that simple it wouldn't be so bad, but you would need to give access to users who should be able to see forum post nodes... otherwise they wouldn't be able to see the forum terms...

I still think keeping the terms public as they are from core is perfectly fine... as long as the nodes themselves are private...

salvis’s picture

Exposing the name of a private forum to the public is not OK (if that's what we're talking about here).

pumpkinkid’s picture

@salvis in order to see the forum name, it needs to be displayed to the user in some form... For instance... if I have a forum called "Super Secret Forum" and a user doesn't have access to it, they will never see it exists... However, if they know it exists, they can follow the steps in #21 and it wouldn't matter if they see the term name, they already know it's there...

The big issue, as far as I see it, is that currently, according to @lirantal, *NOT* having forum posts as a Standard Group Node allows the unauthorized user to view the private forums after knowing what the term is and following the steps in #21... The reason I think this small detail matters, is because, in my opinion, the OG Forum Module *SHOULD* support forum posts as both Standard Group Node and Not.

If the term is private, then any forum post with that term is going to be private regardless of the node access settings... For instance, if I have a public facing forum post under a forum called "Support" but I have another "Support" forum under my "Super Secret Forum"... both forum posts will be private due to the "Support" term being set to private... and we wouldn't necessarily want that...

salvis’s picture

However, if they know it exists

What if they just try tids at random?

Will they see the secret forum in the forum selection list when they create a forum post?

I don't know the concept of private terms — I'll bow out shortly, because I really don't know enough about OG.

In your example with the "Support" term, is the term in a free tagging vocabulary or in a hierarchical one? In the latter case there could very well be two "Support" terms with different parents, and they would be different.

One last thought: I've had little contact with the Security Team, but convincing them that this issue is resolved may require writing Simpletests. It would certainly help. It's a lot of work, but if you'll need to do it anyway in the end, you might just as well do it now, and it would help you to clarify and lock down the expected/required behavior and to evaluate the various approaches in an objective way.

pumpkinkid’s picture

@salvis I see your point about the terms, I hadn't thought about that and I'm not sure what the answer to that is.

That being said, I agree with the simpletest writing and will definitely start working on that.

jean84’s picture

"@salvis in order to see the forum name, it needs to be displayed to the user in some form... For instance... if I have a forum called "Super Secret Forum" and a user doesn't have access to it, they will never see it exists... However, if they know it exists, they can follow the steps in #21 and it wouldn't matter if they see the term name, they already know it's there... "

"What if they just try tids at random?"

I think the question would be:

1.) what will the security team say, that the only question at all here i think.

2.) If the user trys tids for ogforum and it give back forum names is this a design flaw of ogforum or would the same happen to the normal forum? So if it would be a heritage of the normal forum the og forum should not be responsible to fix it, the normal forum should be responsible to fix it.

3.) i think this issue was not the main reason why og forum was abandoned, the main reason might be the sql attack.
So the ogforum might also be unmarked as abandoned before this problem is fixed as it looks more like a minor security issue.

4.) maybe it might be a good idea to make some dialog with the resposible of the security team. Because the og forum is now in its abandoned and unpatched state a major security risk because many people stated that they are still using it beside the sql attack possibility.
Means this sites are actual in quite danger especially as this problem was made public anyone who knows about that problem and know a site where ogforum is still running could get adminrights with this bug.
At this point i also think the security team is in the responsibility to find a solution as the solution to just abandon it and forget it is a major security risk itself.
From this point of view would i suggest to delay the minor problem and bring out an update to fix the major problem first and later this problem can be discussed again.

lirantal’s picture

@jean84 - what? that just doesn't make sense. if the terms are only accessible for administrators then how would you display the forum list (which are the terms in effect) to users?
my patch does exactly this job of restricting access to the terms based on whether you're a user of the group, hence you have access to it or if the forum was set to public.

@pumpkinkid - "I still think keeping the terms public as they are from core is perfectly fine... as long as the nodes themselves are private..." - while that is what you think this is exactly the reason why this module got the SA and removed from drupal official repo. If that's what you think then keep on using this module as you will, I don't see what other interests you have in fixing this module as at your core you don't think it needs any fixing and you agree this is an ok-behavior.

Regarding the issue of standard group post or not - I didn't test the situation of the forum topic content type not set to 'standard group post' so I don't know if my patch fixes it or not. Try and see.

So you're bringing up the issue again of whether seeing the forum and group term is a security concern or not and I don't see why. The security team has decided that it is and I agree, are you really going to waste time convincing them otherwise? Seriously, come'on.

Please take some productive actions rather than running around the tree.

Regards,
Liran Tal.

jean84’s picture

Okay does this mean that all of the three issues are fixed now or not?

Lirantal im not so deep inside the stuff as you three.

If the issues are fixed then the plan is easy making the simpletest and present the results to the security team

lirantal’s picture

@jean84 - not sure how you counted 3 issues? I count only 2 issues in the SA. One was fixed already a while ago and the 2nd is my fix.

Please feel free to apply my patch to ThaPanz's repo and provide feedback/unit tests.

Regards,
Liran Tal.

pumpkinkid’s picture

@lirantal I am sorry you feel that I'm wasting time... however, because Drupal is open source and community maintained, we must agree on how to correct this issue before moving forward. I'm not trying to convince the security team of anything just yet, just trying to discuss the possibilities for a fix that works for both case uses.

Rather than being negative about the discussion we are having, you should appreciate that we are working towards finding the solution.

lirantal’s picture

@pumpkinkid - I'm not being negative but the vibe that I'm getting is that you're trying to drag this module down to hell while I'm trying to take it out of it.

While you're saying "just trying to discuss the possibilities for a fix that works for both case uses." - there is a fix already and it's been hanging around for more than a month, yet no one is taking action. There's a fix. That's it. who tested it? we got no feedback.

I don't see how you are working towards a solution.
There is a solution already. Did you test it? is it not working? is there something missing?

pumpkinkid’s picture

@lirantal I have not had a chance to test your patch, you are absolutely right about that, but you yourself have said that you have not tested the forum posts not set as Standard Group Nodes either.

I imagine that the same way that I have not had a chance to test your patch, you have not had a chance to test your patch with my use case... I am not trying to drag this module to hell, I need this fixed as quickly as possible as well as everyone else here. However, just because your patch works for one use case, doesn't mean it works for all... which is the point I brought up on #145... ("I believe that we need to agree on whether or not the two use cases are going to be covered by this module or decide on if we need to force the forum posts to to be Standard Group Nodes and move forward.")

I am only continuing the discussion because of the fact that otherwise there wouldn't even be a discussion. It would simply be left, as you said, untested for months... I trust you and believe you when you say the patch works as you describe, but by your own admission, we do not know if it works as I have stated.

Regardless on whether or not you agree that my use case should be supported by this module or not, the fact is that we would still need to force the forum posts to be Standard Group Nodes before we can call the patch *the* fix... To decide if we should do that we need to discuss the boundaries of the OG Module and make that decision.

Ideally we would have more people chiming in on this as I would hate to be partly responsible for applying a patch to this module that would render Drupal sites currently using OG Forum un-usuable.

lirantal’s picture

@pumpkinkid I did not test the content type set to anything else than standards group post because I don't have any interest in doing that since I don't consider that a valid use case. With that said, we don't even know yet if that's an issue or not (I believe that it is but have not tested it).

Drupal Commons distribution is definitely a role model here since it makes use of OG as it's core component and supports many other content types like wiki, blogs, discussions, etc and they are all, with no exception, set to be standard group post. OG Forum is no exception in this regard.
This module by definition is supposed to provide forum support on a group level.
If you want anything other than that feel free to install one of the site-global forum modules and use it instead or in parallel.

I care about productivity and continuing discussing either issue here without actual feedback is a waste of time in any way that you look at it. We can keep arguing for months if the SA is really a security issue or if standard group posts is the right way or wrong way. Instead, if you have an interest in the latter for instance, go ahead and test it and come back with feedback and patches. Open source is nothing new to me and in all of the projects that I have founded and was involved in I always put myself available to anyone for collaboration but it requires you to be pro-active and actually bring something to the table.

And I'll repeat this once more for the 5th time probably: make a drupal install, perform the install of the module from ThePanz repo which includes some fixes along with the 1st part of the SA patch, then apply the patch I provided and install OG Forum as you should. Test it.
If you need help, have questions, would like to debug or want to sit on a beer with me - you're welcome, I'll give you my email, my gtalk or whatever and we'll work out the issues together. Understand that this is a collaborative work hence you can't expect to just speak your mind, you actually have to take action, whether it's actually testing or doing dev work.

Regards,
Liran Tal.

pumpkinkid’s picture

@lirantal I have been testing, and have been reporting my findings... Just because I haven't had time to test *Your* contribution, doesn't mean I haven't been contributing my own feedback... just look at the thread...

That being said in an effort to stay on topic I will no longer discuss the use case issue with you as you've made it clear that you don't agree that it is valid. I appreciate that, but to continue down this road back and forward isn't going to fix this, and making my case is preventing me from catching up on work I need to finish before I can sit down and test your patch.

Open source isn't new to me either, which is why I'd like to make sure I'm in the minority when it comes to how I'm using this module before we find out the hard way.

nhoeller’s picture

First of all, kudos to the folks that are trying to fix this problem! I use OG Forum to control forum access in a multi-group environment but other commitments have prevented me from focusing on this issue. I am just wrapping up a Drupal upgrade (only one more bug to fix, I hope) and will have time to do some testing next week.

I will reproduce the problem with 6.x-2.2 using the steps in post #21, rebuilding node permissions if required. I will dump the node_access table for documentation purposes. If I can reproduce the problem, what patches should I be testing against what code base?

As I understand Drupal, the 'Standard group post' is defined at the Forum topic content type. I can create two additional content types based on Forum topic and test what the code does if "May not be posted into a group." and "Wiki group post (any group member may edit)."

Let me know if this course of action makes sense.

pumpkinkid’s picture

@nhoeller #123 contains the latest patch we need to do testing against. Please use the @thePanz's repo to perform your testing.

Thanks for wanting to lend a hand.

jvieille’s picture

Is it possible to have a patch against the last dev on Drupal.org or a fully functional updated module?
Working on another repo might explain the lack of significant testing from the community.
As for myself, I really don't know what to do from there
http://drupal.org/sandbox/thepanz/1110446

Thanks

nhoeller’s picture

I have done some preliminary testing using OG Forum 6.x-2.2 using a copy of a production Drupal environment. I had created a Members group in OG with the options:

  • Moderated
  • List in groups directory
  • Private group

Forum topics are defined as Standard group post.

Options chosen for OG Access Configuration:

  • Visibility chosen by author/editor using a checkbox on the posting form. Checkbox defaults to private.
  • Group administrator chooses whether her group homepage and audience are private or not. Defaults to private.

None of the following options were checked OG Forum Configuration:

  • Would you like all group forums to be placed in the same container?
  • Automatic forum publicity
  • Allow public choice
  • Make all forums public

I do not recall having clicked on the two buttons:

  • Publicize old groups
  • Switch to auto mode

I do not have any public forums in the Members group (term=15). I tested with a forum (term=212, forum=212) and a forum topic (nid=644). The node_access table for nid=644 showed og_admin=1/1/1 and og_subscriber=1/0/0 (grant_view/grant_update/grant-delete). I have not done a node access permission rebuild yet.

I logged out and tested the following access:

  • Access to taxonomy term 15 (Members): displays group name and "There are currently no posts in this category."
  • Access to taxonomy term 212 (private forum): displays forum name, description and "There are currently no posts in this category."
  • Access to node 644 (private forum topic): "Access Denied"
  • Access to forum list (step 3 in post #123): only showed public forums
  • Access to forum 212 (step 4 in page #123?): "Access Denied"

I assume test #1 in post #123 should include the term number of the forum topic. I am not sure what the "+" in test #2 represents.

So far, it appears my results are consistent with what others are seeing, except for visibility of the private forums (test #3 in post #123). Before I go further, are there any other setup changes or tests that I should try?

nhoeller’s picture

Just in case having some public forums/forum topics within an OG group makes a difference, I created a new forum but did not see any way of identifying it as public. I created some forum topics and identified them as public. The node_access table showed the same entries as above and in addition og_public=1/0/0 (gid=0).

When I logged out, I was able to view the public forum topics. However, displaying the forum list only showed forums outside the Members taxonomy hierarchy. This is not consistent with the results of step 3 in post #123.

At this point, I am not clear on how relevant the node access permission rebuild issue is to this specific problem. I am inclined to hold testing this until later.

Unless I hear suggestions about further test cases, I will replace the existing OG Forum files with those from https://github.com/thePanz/og_forum/, apply the patch at http://drupal.org/files/issues/og_forum-resolve-security-issue-1055424-1... and re-test.

pumpkinkid’s picture

@nhoeller The rebuild issue would still be relevant, if the results you found are altered when the permissions are rebuilt... For example, (if I followed your testing correctly) I would expect term 212 to actually show the forum posts after the permissions have been rebuilt...

If this is not the case then the only other variable is if the rebuild issue occurs when the nodes are not set to Standard Group Node.

nhoeller’s picture

@pumpkinkid, I can certainly test node access permission rebuild. However, this issue seems distinct from that described in the original security report. Based on my understanding of #237634: Rename node_access_write_grants() to _node_access_write_grants() and discourage its use, permission rebuild seems to be a more basic problem in the way the node_access table is managed when multiple contributed modules are setting permissions for the same node.

For the moment, I am going to assume that most people using OG Forum have defined the Forum topic content type to be "Standard group post" or possibly "Wiki group post". Setting it to "May not be posted into a group." seems to negate the purpose of implementing OG Forum. I am not sure how to create a new content type that duplicates Forum topic but with different settings.

One of the challenges is identifying all the possible configuration options and usage scenarios, then testing each one.

pumpkinkid’s picture

@nhoeller All right well I seem to be in the minority as to how I use the module so I will now bow out of this discussion.

Although the discussion of Standard Group Node or not sprung up after I had been testing the module, and although I still feel that the issue described in the SA *IS* related to the rebuild issue, I am tired of trying to convey my point. I can clearly see that I am outnumbered in my thinking and I rather not get in the way of a fix if that's all I'm doing.

nhoeller’s picture

@pumpkinkid, I will take a backup of the database, test out the latest patch, revert back to the original database/code, rebuild the permissions and repeat the process.

I re-read your post #136. What exactly do you mean by having "forum posts work outside of the Organic Group functions as well as within them"? It is quite possible that I do not understand exactly what the "Standard group post (typically only author may edit)." option does. I have forums that are accessible to users who are not members of any group, using the same content type as forums within groups.

nhoeller’s picture

I had trouble installing git on my Linux server so installed a copy on Windows, cloned git://github.com/thePanz/og_forum.git, created a tar file and installed a clean copy of thePanz og_forum code. I had trouble installing the patch from http://drupal.org/files/issues/og_forum-resolve-security-issue-1055424-1... -

patch < og_forum-resolve-security-issue-1055424-123.patch
patching file og_forum.module
Hunk #1 FAILED at 115.
Hunk #2 FAILED at 295.
2 out of 2 hunks FAILED -- saving rejects to file og_forum.module.rej

I could see no obvious issues with the line numbers and all the lines appeared to match, so I manually applied the changes. I did an update.php but no database changes were required.

I cleared the cache and did the previous tests logged into my own account (site administrator privilege, member of the Members group. I also added a test for taxonomy term 8 since I had crated a public forum topic inside the private forum associated with taxonomy term 15.

  • Access to taxonomy term 15 (Members): displays group name and "There are currently no posts in this category." (2011/07/26 this response is consistent with 6.x-2.2)
  • Access to taxonomy term 212 (private forum containing a public forum topic): displays forum name, description and the forum topics
  • Access to taxonomy term 8 (private forum): displays forum name, description and the forum topics
  • Access to node 644 (private forum topic): displays forum topic
  • Access to forum list (step 3 in post #123): "Page Not Found"
  • Access to forum 212 (step 4 in page #123?): displays forum topics within forum
  • Access to forum 8 (step 4 in page #123?): displays forum topics within forum

I then logged off and repeated the tests.

  • Access to taxonomy term 15 (Members): "Page Not Found"
  • Access to taxonomy term 212 (private forum containing a public forum topic): displays forum name, description and the public forum topic
  • Access to taxonomy term 8 (private forum): "Page Not Found
  • Access to node 644 (private forum topic): "Access Denied"
  • Access to forum list (step 3 in post #123): "Page Not Found"
  • Access to forum 212 (step 4 in page #123?): displays public forum topic within forum
  • Access to forum 8 (step 4 in page #123?): "Access Denied"

In terms of anonymous access, the patch appears to block leakage of private information to anonymous users. Creating a public forum topic within a private forum will cause leakage of the forum name and description. I would personally prefer "Access Denied" over "Page Not Found" for consistency. Also, in my setup, "Page Not Found" does not have any sidebars - some users may not know how to login from this state.

I reverted back to the un-patched thePanz code and reproduced the incorrect result when a group member tries:

  • Access to forum list (step 3 in post #123): "Page Not Found"

As others have suggested, would it make sense to create versions of the CSRF and lirantal patches that apply to the 6.x-2.2 code? It will make debugging easier and may also reduce the amount of work required to convince the Drupal Security Team.

I will do some testing later this week to determine the impact of a node access permission rebuild and whether thePanz/lirantal code solves any problems in this area.

thePanz’s picture

@nhoeller: thanks for your time spent testing my forked module and other patches. I don't know your Drupal/PHP experience, but what about writing few tests using SimpleTest (http://drupal.org/project/simpletest) API for OG_forum?
This could provide some automated tests for OG_Forum development and permission/access checks..

Regards

nhoeller’s picture

Update to post #171: both the 6.x-2.2 and thePanz versions of og_forum return the same result when an authorized user tries to access a taxonomy term at the group level In my case, taxonomy term 15 associated with the Members group). I can get a forum list on 6.x-22 by accessing http://(domain)/forum.

I will update post #171.

nhoeller’s picture

@thePanz, based on an initial reading of SimpleTest, my PHP skills are not up to par. I nmight be able to add specific tests once the general framework is built.

nhoeller’s picture

@pumpkinkid, I restored my Drupal database and reverted to the 6.x-2.2 version of og_forum. Unfortunately, a node access permission rebuild did not not appear to change anything. I still get 'Access Denied' when I try to access a private forum topic. I checked the node_access table before and after the permission rebuild - in both cases I see

nid 	gid 	realm 	grant_view 	grant_update 	grant_delete 
644	119	og_admin	1	1	1
644	119	og_subscriber	1	0	0

Based on what I have read, I suspect node access permision rebuild only causes a problem if there is another contributed module that sets node permissions for the forum topic. I specifically implement Organic Groups and OG Forum to avoid requiring multiple node access modules.

nhoeller’s picture

@pumpkinkid, any suggestions on what I should test in terms of the forum topic Organic Group settings? What should I expect to happen if I enable "May not be posted into a group"? I can test "Wiki group post" - that might have an impact on access permissions.

pumpkinkid’s picture

@nhoeller the testing that resulted in the rebuild issue is noted on #86. It's been a while since we went down that road, but the issue comes apparent when "Organic groups access control" is enabled.

The problem (I thought) is that as mentioned in posts like #67, some sites don't have that issue, presumably because they don't have the "Organic groups access control" module enabled.

Media Crumb’s picture

So with all this back and forth are lirantal changes solid?

nhoeller’s picture

@pumpkinkid, I have Organic Groups Access Control installed and configured. I have no other node access modules installed. So far, I have not been able to reproduce the permission rebuild problem.

How have you configured Organic Groups Access Control? Also, what is the Organic Groups setting for the content type Forum topic?

What triggers updates to the node_access table when a private forum topic is created? As far as I can see, a forum topic appears to be a regular node and Organic Groups Access Control is designed to control access to nodes within a group structure.

nhoeller’s picture

@Media Crumb, thePanz code with lirantal's patch does not leak information about private taxonomy terms to anonymous users for the test cases described in posts #21 and #123. At the moment, I am trying to reproduce a potentially related issue where private forum topics become public after a node access permission rebuild.

In my test environment, thePanz code breaks http://(domain)/forum even for site administrators. It is possible that I did not install the fixes properly or that this issue is unique to my environment.

OG Forum has a lot of other functionality. For example, I have not tested with forum containers, nested forums or changing the private/public state of a forum/forum topic. There are also settings and buttons in the OG Forum configuration that I personally do not fully understand and have not tested. I hope that there is an existing set of test cases for OG Forum.

pumpkinkid’s picture

@nhoeller To be clear, the tests I performed were all on the same test installation, not on any of my actual sites. That being said I have sites that set the "Organic Groups Usage" differently. The test site I created did use the "Standard Group Node" setting as it is the most common use case.

Organic Groups Access Control is set to Visible only within the targeted groups and Group Administrator chooses... Defaults to public.

nhoeller’s picture

@pumpkinkid, how did you configure the Organic Groups node for the group you created for testing? What are the 'Groups' setting for your Forum topic?

I will free up some time and build a minimal Drupal test environment with your settings. It will help with confirming whether the issue with http://(domain)/forum is unique to my environment. I will capture the build steps for input into creating a SimpleTest.

pumpkinkid’s picture

@nhoeller Moderated | Private Group

nhoeller’s picture

@pumpkinkid, I have set up a minimal test environment. I enabled Forum, OG Forum, Organic groups and Organic groups access control. I gave authenticated users all Forum, OG module and OG Forum permissions. Forum and Page were defined as Standard group post. I created the content type Organic Group (group node) and a node Members of type Organic Group:

  • membership=Moderated
  • Private group

I accepted the default Organic groups access configuration:

  • Visible only within the targeted groups
  • Group administrator chooses whether her group homepage and audience are private or not. Defaults to public.

I created two forum topics under Members and the default General discussion. I was not able to set any of these forum topics as public, probably due to the Organic groups access configuration. I then created two users, one of which was assigned to Members while the other was not. The user in the Members group had full access. The non-Members user was able to see the forum list (Members group and the Group discussion forum) but had no visibility to forum topics. An anonymous user could not see the forum list, got Access denied trying to directly access the Members forum container or the General discussion forum but could get information by viewing the taxonomy terms.

I tried doing a node access permission rebuild. The og_admin and og_subscriber settings in the node_access table did not change, nor was an anonymous user or non-Member user able to see any of the forum topics. Not sure what is different about our environments.

My plan for tomorrow is to test out thePanz code and then apply the lirantal patch.

pumpkinkid’s picture

@nhoeller Sigh, I'm not sure either... the only thing I'm thinking is if you think it would help, I could package the whole installation and link to it...

nhoeller’s picture

@pumpkinkid, I was not able to reproduce the node access permission rebuild issue after installing thePanz code and then applying the lirantal patch. it would be great to determine whether this is really an OG Forum problem.

@thePanz, I verified in the minimal test environment that http://(domain)/forum results in a Page not found error, even for the site administrator or users who are members of the group. Access to individual forums using http://(domain)/forum/# works properly.

@lirantal, testing of your patch in the minimal test environment was consistent with what I reported earlier. An anonymous user is getting both Access denied (attempts to access specific forums) and Page not found (attempts to access the forum taxonomy terms). Is there a way return Access denied in the latter case?

I did a test with an authenticated user who is not a member of the group. Access to a specific forum using http://(domain)/forum/# returns the title Forums with no forums or forum topics listed. As had been mentioned before, there is a Post a new fourm topic link but the Forums pull-down list is empty.

I am having trouble keeping track of all the test results. I will consolidate all the tests into an Excel spreadsheet.

nhoeller’s picture

FileSize
19 KB

I have attached a spreadsheet showing the configuration of the minimal test environment and the results of testing. I highlighted results that differed from the previous test in the spreadsheet. The results are consistent with posts #165 and #171 with the exception that they included tests with private forums containing public forum topics. The minimal test environment does not give me the option of marking a forum topic in the Members group as having public visibility.

Is there a description of how OG Forum works? I know I can read the code, but my PHP skills are very limited - I have trouble even creating some sort of a structure map of the module. It would help in building tests cases to understand how OG Forum interacts with other modules such as Taxonomy and OG. Based on an initial look, the number of combinations of OG Forum settings and actions seems very large.

nhoeller’s picture

@thePanz, I added some traces to og_forum.module to figure out why calls to http://(domain)/forum were returning Page not found. The way I read the 6.x-2.2 code, the og_forum_topic_redirect function will use a default value of $tid=0. If $tid is empty, the function will build the page showing the list of forums.

In your code, if $tid is empty, it appears the call to forum_page($tid) is returning a value of 2 in all cases.

I added the following lines to the og_forum_topic_redirect function just before return forum_page($tid);

  } else {
    $tid = 0;
  }

Calls to http://(domain)/forum are now sort of working (:-).

@lirantal, anonymous users get No forums defined but authenticated users who are not part of the Members group see Members / General discussion.

nhoeller’s picture

I see the challenge: forum_page is a Drupal API that does not take into account group affiliation or private/public forums. Beyond this point I would definitely be getting into water well above my head (:-).

nhoeller’s picture

In the interests of moving this project along, I am willing to install the latest code on one of my production sites. I think I can get around the http://(domain)/forum problem by copying the forum_page code into the og_forum_topic_redirect function and recursively calling http://(domain)/forum/(number). If there is a better/cleaner way, please let me know.
Thanks, Norbert

Skirr’s picture

Is there any progress ?

jvieille’s picture

Very interested too

nhoeller’s picture

@Skir/@jvieille, unfortunately, project deadlines and limited skills have prevented me from making further progress. I was hoping that more experienced people would 'weigh in' now that summer vacations are over. I may have some cycles late in September or early October.

Media Crumb’s picture

Any news?

lightsurge’s picture

Maybe a better bet than fixing this module would be to integrate advanced forums into OG much as @that0n3guy did here and still works on Drupal Commons:

http://commons.acquia.com/wiki/using-forums-drupal-commons

Edit: actually, this will surely be subject to the same issue in terms of public taxonomy... to be honest I don't see what's different between this and a shared tagging taxonomy for any content on an OG site as Commons uses, surely the problem is for both. For example if you can tag blog posts in a private group using a public taxonomy, I don't see how that's different.

Edit: okay now I do, the difference is node access, so advanced forums does look to be a good option.

nhoeller’s picture

@lightsurge, have you had a chance to test out the Drupal Commons alternative to OG Forum? I like the idea of delegating forum access to the underlying OG feature. The integration with Advanced Forum is a nice feature - I recall seeing some comments that Advanced Forum and OG Forum did not get along well.

Unfortunately, Commons 1.6 appears to have broken the code, there have been no code updates since January or any comments more recent than 20 weeks ago. A query around that time about whether the code could replace OG Forum did not get a response.

nhoeller’s picture

@pumpkinkid/@thePanz/@lirantal, I finally have some time to work on OG Forum again. The OG Forum issues list has been quiet except for people trying to find out the status. Based on the results of my testing, are you comfortable that the security issues with OG Forum have been resolved? I have refreshed my test environment and will try to resolve the 'Page not found' when display the forum list. If this last issue is resolved, are we any closer to finding an owner for OG Forum? I am in no position to volunteer - my PHP skills are rudimentary on a good day.

lightsurge’s picture

The Commons advanced forums feature worked fine, its main premise is providing one forum per group and then non-OG (i.e. public) forums, I would imagine it could be easily adapted to provide the same functionality for OG generally if a programmer were interested.

In the end I just used advanced forums with OG, I didn't need anything else. While advanced forums can take over core forums and work in the same way but more 'advanced', what it also does is provide a Views style and a variety of extensible themes to make nodes look like topics. So for me, each of my organic groups have their own forum which is essentially just a view limiting topics to the group in context... and there's also a public forum.

For the universal forum listings I group forum posts by organic group names, and so I don't actually need to use taxonomy at all. Views can group nodes into 'forums', which could be any field accessible to Views, be that by taxonomy term, a CCK field or by Organic group.

Perhaps there's a module which would allow a 'subgroup' in Views (i.e group by OG name, and then by a CCK field)... so that rows are first grouped by a primary field, and then grouped by a secondary field, that way you could have multiple forums per group. I couldn't find one but don't really need one right now. Perhaps you could use the OG subgroups module.

Media Crumb’s picture

Im a bit confused by this. So you used OG Forums or created a new node type for groups and then just styled them as forums?

lightsurge’s picture

The latter, I created a node type for forum topics and just styled them as forums. Advanced forums provides a plenty of extensions to help you along the way.

If you wanted multiple forums per group you could add a cck text field of type 'forum' to the forum topic, and then using Views group forum topics into forums. The contents of these fields would then be protected by node access restrictions.

The Commons feature I linked to provides similar behaviour to og_forums but it still uses global taxonomy for forum names, just as the core forums module does. So presumably it's still subject to:

OG Forum stores private group and forum information in a global vocabulary, which can lead to information such as group and forum names being disclosed to members not part of the private group.

lightsurge’s picture

Core taxonomy in 6.x has so many weaknesses it makes it a can of worms for projects like these. So many access/moderation/management gripes. The patched version here seems a reasonable workaround for access control to terms, but the possibility of exposing forum taxonomy to non-priveleged users would still have the potential to re-appear as soon as an unsuspecting administrator installs some other contrib module dealing with vocabularies.

If you need 'containers', isn't using http://drupal.org/project/nodehierarchy a reasonable alternative?

lightsurge’s picture

I created a feature request in advanced_forum if anybody's interested #1328176: Integrate Node Hierarchy

Media Crumb’s picture

So i take it this project is DOA? Are there any ways to use group forums at all in a secure fashion?

Michelle’s picture

Not yet, as far as I know. I'm still plugging away at Artesian but it's still going to be a while before it's ready for production use.

Michelle

nhoeller’s picture

I have finally freed up some time to see if I could fix the problem with the secure OG Forum code when displaying a list of forums. Unfortunately, upgrading to og-6.x-2.2 causes OG Forum to 'white screen' (see http://drupal.org/node/1427566). The problem was not introduced by thePanz or lirantal code - it can be reproduced in og_forum-6.x-2.2.

jvieille’s picture

Thanks a lot, this is much expected!
I love OG Forum, and any suggested alternatives do not match it.
I disabled it for some time, but using it again despite the evil warning

Media Crumb’s picture

Hey nhoeller,

Did this fix at http://drupal.org/node/1427566 clear things up? Also, may I ask what the latest version of OG Forums you are currently using is? I'm assuming its thePanz code base?

nhoeller’s picture

Media Crumb, the fix will allow me to continue working on the OG Forum security problem but I do not see it as a permanent fix unless someone can convince the OG folks that there is a bug in their code. My knowledge of how OG Forum interacts with OG is virtually non-existent, so I am not going to be much use.

I have been working with thePanz code base and the lirantal patch. The main problem I am trying to fix is the inability to get a list of all forums that the user has access to (see post #171). I recently noticed a problem where the groups associated with a forum do not show up in the Groups section when editing the forum.

Media Crumb’s picture

nhoeller,

Thanks so much for the reply. I was really hoping OG forums could be used in production but it looks like things are fairly far off. Is there a version of thePanz code that is the most up to date? I'm not really used to git and I'm more of a download zip file and upload FTP type. At least I can install it and possibly hired out the work.

nhoeller’s picture

Work on addressing outstanding issues with thePanz+Lirantal code has been slow due to the complexity of the code and my limited skills. I have been testing xdebug and have figured out a way to boil down the voluminous output into a more concise trace of key functions with arguments (but unfortunately not function results). I have built a stripped-down test environment with og-6.x-2.2 and og_forum-6.x-2.2 (with the infinite loop patch) and am capturing 'baseline' function traces.

The next step is to install the latest thePanz+Lirantal code, repeat the function traces and attempt to determine the cause of the following issues:

  • thePanz code prevents display of a list of forums that the user is authorized to see (may be related to problems displaying documents by taxonomy tags)
  • when editing forums, the associated groups are no longer displayed after the Lirantal patch is applied

I have stumbled on a number of other og_forum anomalies on my production system that I have so far not been able to reproduce in my test environment. I suspect that the problem may be related to having defined forums prior to implementing og_forum and subsequently moving the forums to the correct group hierarchy. I hope these are just inconsistencies in the database that can be easily resolved.

EmanueleQuinto’s picture

Given that here @sfsu we are using Open Atrium as internal groupware and our users need a proper forum we managed to package everything in a feature following the approach used for Drupal Commons.

It's currently in the sandbox but we're already using in (pre)production: Atrium Forum.

Media Crumb’s picture

Would this work with Pressflow and Normal Drupal 6 installs as well? I'm assuming it would.

EmanueleQuinto’s picture

Well the feature works for Open Atrium (and Pressflow).

You can probably retrofit to plain vanilla Drupal 6 (with OG, context/features and NAT) keeping only the visibility/permission part to solve the issue.

Media Crumb’s picture

I'm currently on a the version 6.X of pressflow so I'll give it a shot. Does it differ in anyway then OG Forum does?

vegantriathlete’s picture

Issue summary: View changes
Status: Needs work » Closed (won't fix)