If request to entity containing field-based gallery produces 403, the request to the Juicebox XML shoud do the same [#2055139]

I just realized that any view restrictions placed on nodes/entities are not fully checked when responding to a request for any Juicebox XML that may be linked to that same entity. So access to an entity with restricted view access may trigger a 403, but access to the related Juicebox XML will still go through normally. This could only impact users that use some method of restricting view access to entities that contain Juicebox galleries (e.g. some form of contrib module like content access, having nodes in unpublished states, etc.).

Ultimately this means that even if a user is blocked from viewing a node/entity that contains a Juicebox gallery, they could theoretically "guess" what the URL is to that entity's related Juicebox XML, and could then view a bunch of Juicebox metadata that they should really not have access to. This metadata is not particularly sensitive but it could contain some title and caption details that are embedded within an imagefield on the restricted entity. If the images themselves are protected (with Drupal's private files feature or similar) access to the XML would still not grant access to the images, so this problem cannot lead to exposure of any protected image data.

So technically speaking this should qualify as an "access bypass" security issue. However I don't believe that it calls for any kind of formal security advisory as there is not currently a formally packaged stable release available for Juicebox. The most recent release is RC2, which according to security policies does not warrant an advisory. What's more I think this would be classified as a "not critical" risk level, with a very minor impact that would not effect many (if any) real users. Because of this I'm handling it here in the issue queues as a normal bug report.

I'll try to fashion a patch ASAP.

Comment	File	Size	Author
#1	juicebox-check_entity_access-2055139-1.patch	1.58 KB	rjacobs

Comments

Comment #1

rjacobs commented 2 August 2013 at 16:34

Status:

Needs review

» Active

Status	File	Size
new	juicebox-check_entity_access-2055139-1.patch	1.58 KB

This trick with this is that there is no universal method to programmatically check access for entities when that check needs to be done independent of normal Drupal menu routing and rendering for an entity. Nodes can use node_access(), but other entity types have their own process or callbacks. It may be that just having a check/switch that tries to catch as many types as possible (and trigger the appropriate logic based on the type) will be the way to go. For all practical purposes this should be fine as I suppose 99.9% of the people who would us the field formatter of this module would be using nodes anyway, and we certainly have a check for those.

Attached is a patch that's a work in progress. I want to experiment with this on a couple different environments.

I also want to see how tools like the services module deal with this, as they also have this problem of programmatically checking access to various entities outside of Drupal's normal rendering interfaces.

Comment #2

rjacobs commented 2 August 2013 at 16:28

Status:

Active

» Needs review

I should also note that I added a post in the "Module Development and Code Questions" forum about this just to see if anyone else had any expertise on the best way to address this kind of access-check problem:

https://drupal.org/node/2055927

I'll hang on a bit to see if any comments come in there (or here). If I don't hear anything I'll probably proceed with the general logic outlined in #1

Comment #3

rjacobs commented 6 August 2013 at 03:48

Status:

Active

» Fixed

After additional research and testing I have proceeded with a commit that is a slight variation on the patch from #1. This is captured here:

http://drupalcode.org/project/juicebox.git/commit/1b5c9d9

This fix will ensure that the same entity level permission that are enforced when the gallery page is viewed are also enforced when the XML is requested. Given that each type of entity can have its own access checking methods I can only include support for common core entity types along with some "blanket" support via the Entity API (for those that have it installed, which will only be applicable in special cases). For any cases where a definitive access check cannot be run, we default to "Access Denied" on the XML request. I'll add some additional documentation about dealing with any outlier cases where this can cause problems as soon as the next release is packaged.

Even though this does not qualify for a security advisory I'll still try to get the next release tagged with the "security update" taxonomy term on d.o. so that active users get a local site notice.