Since in the security_review.inc file the review callbacks start with a _ (e.g. _filefield_security_checks and _views_security_checks) they don't count as hook implementations and thus

$function = $module . '_security_checks'; // Call the hook implementation.

will not find them in security_review_check_help(). Whats the best fix for this? Removing the prefix would help, but they take the modules namespace.

CommentFileSizeAuthor
#1 security_review-check-help-1361640-d6.patch2 KBcoltrane
#1 security_review-check-help-1361640-d7.patch2.04 KBcoltrane

Comments

coltrane’s picture

coltrane
he/him
commented
Status: Active » Needs review
StatusFileSize
new security_review-check-help-1361640-d7.patch2.04 KB
new security_review-check-help-1361640-d6.patch2 KB

Good catch! Fix would be to just get all checks and use defined callbacks.

doublejosh’s picture

doublejosh commented

These patches seem to be created oddly with /a and /b folders. Hadn't seen that before, and it doesn't apply the patch.

diff --git a/security_review.module b/security_review.module
index 1c2c53b..1685743 100644
--- a/security_review.module
+++ b/security_review.module

How do I make use of it? I just applied the code changes (works great).

charles belov’s picture

charles belov
he/him
Primary language English
Location San Francisco, CA, US
commented

For clarification, would this explain why [path to site]/admin/reports/security-review/help/views/access gets a Page not found? Or do I need to file a separate bug?

doublejosh’s picture

doublejosh commented

This patch fixes that problem.
I had trouble here because I was applying the patch wrong.

REALLY needs to get rolled in, it's breaking the basic workings of the module.

snufkin’s picture

snufkin commented
Status: Needs review » Reviewed & tested by the community

Since it fixes the issues setting it to rtbc.

coltrane’s picture

coltrane
he/him
commented
Status: Reviewed & tested by the community » Fixed

Committed! http://drupalcode.org/project/security_review.git/commit/1312770 and http://drupalcode.org/project/security_review.git/commit/db41810

Will be in dev releases only for now.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

davidi’s picture

davidi commented
Status: Closed (fixed) » Active

Is there any chance this bugfix could be pushed out to a new recommended release? We are keen to use this module but in testing found this problem with filefield and view access checks. Discovered this issue and was hoping to get the fix but don't want to go with an dev release. Thanks!

coltrane’s picture

coltrane
he/him
commented
Status: Active » Fixed

@davidi, I hope to make a new Security Release soon. You can help by reviewing the issues in the queue http://drupal.org/project/issues/security_review?categories=All thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.