On my site, I have some group names with apostrophes, ampersands, and other HTML special characters. Because OG Menu runs a check_plain on the block title, these titles read something like the following when displayed on the site: John Doe's Group, instead of: John Doe's Group.

Line 152 of og_menu.module reads:

    return array('subject' => check_plain($menu['title']), 'content' => menu_tree($menu['menu_name']));

If I remove the check_plain(), the title renders correctly, but it seems the check_plain is necessary here... or maybe not? For now, on my site, since I'm the only one that can edit group names, I've removed the check_plain() so that apostrophes, ampersands, etc. in group names (which display as OG Menu block titles) display correctly.

However, would it be better to use filter_xss or something like that instead? Or, does block.module already check_plain the string?

Comments

geerlingguy’s picture

Status: Active » Needs work
StatusFileSize
new511 bytes

Attached is a simple patch for those who need to have this fixed... however, this patch could introduce a security flaw in your site (I haven't seen whether check_plain() is run on the title later on in block.module or not...

(and, for my own internal tracking... http://archstldev.com/node/886).

jide’s picture

@geerlingguy: Oh yes of course... But we should use filter_xss_admin() here instead, if you could you reroll the patch, that'd be nice.

geerlingguy’s picture

Status: Needs work » Needs review
StatusFileSize
new992 bytes

Patch attached (first patch I've done following the directions on the 'Git instructions' tab!).

Hopefully this method works well - will this keep merge that commit into og_menu correctly?

jide’s picture

Status: Needs review » Reviewed & tested by the community

Thanks geerlingguy !

The best instructions on how to create a git patch I've found is there: http://www.gizra.com/content/using-git-write-patches, although it dates from the CVS era, it explains how to use branches to create patches - and drush make for example, won't accept patches with :

--- a/og_menu.module
+++ b/og_menu.module

Just figured this out a few days ago :)

jpamental’s picture

You may find you have to do the same thing in line 192 (I see that the patch above has been incorporated in the module already)

I was still seeing this happen when the menu was included in the Group Details block but swapping filter_xss_admin in for check_plain in line 192 fixed it right up.

Jason

jpamental’s picture

Interestingly I'm seeing this elsewhere now: in views output (echoing a node title) - wonder if this is a more widespread issue of not updating from check_plain to filter_xss_admin - newer function now being 'more required' by core?

pfrenssen’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Closed (outdated)

Closing, since the Drupal 6 version of OG Menu is no longer supported. If this issue is still relevant for the currently supported versions (D7 and D8) then feel free to update the version and reopen the issue.