Attachments Permission

[jdarwood007]

Description

This is occurring in a customization, but it is due to a bad logic check in SMF. The customization is using SMF's attachment system and using the integrate_download_request hook.

In ShowAttachments.php we have:

    // No access in strict maintenance mode or you don't have permission to see attachments.
    if ((!empty($maintenance) && $maintenance == 2) || (!allowedTo('view_attachments') && !isset($_SESSION['attachments_can_preview'][$attachId])))
    {
        send_http_status(404, 'File Not Found');
        die('404 File Not Found');
    }

The code that is failing is allowedTo('view_attachments')

This calls the allowedTo function, with no $board parameter, which falls back to loading permissions from $user_info['permissions'] which is filled from the {db_prefix}permissions based on the users associated groups. However, the view_attachment permission is defined in ManagePermissions as a "board" permission.

    $permissionList = array(
[...]
        'board' => array(
[...]
            'view_attachments' => array(false, 'attachment'),
            'post_unapproved_attachments' => array(false, 'attachment'),
            'post_attachment' => array(false, 'attachment'),
        ),
    );

As such we can never get the view_attachments to function when called from a customization that isn't being previewed or loaded from a board.

As an interesting test case. In the customization, if I append topic=XXX with a topic that the user can see, they can suddenly see the attachment. This is because loadBoard (In Load.php) will append/update $user_info['permissions'] to add in the board permissions.

    // Get the board permissions.
    if (!empty($board))
    {
        // Make sure the board (if any) has been loaded by loadBoard().
        if (!isset($board_info['profile']))
            fatal_lang_error('no_board');

        $request = $smcFunc['db_query']('', '
            SELECT permission, add_deny
            FROM {db_prefix}board_permissions
            WHERE (id_group IN ({array_int:member_groups})
                ' . $spider_restrict . ')
                AND id_profile = {int:id_profile}',
            array(
                'member_groups' => $user_info['groups'],
                'id_profile' => $board_info['profile'],
                'spider_group' => !empty($modSettings['spider_group']) ? $modSettings['spider_group'] : 0,
            )
        );
        while ($row = $smcFunc['db_fetch_assoc']($request))
        {
            if (empty($row['add_deny']))
                $removals[] = $row['permission'];
            else
                $user_info['permissions'][] = $row['permission'];
        }
        $smcFunc['db_free_result']($request);
    }

Thus I can make the customization's call on the attachment work by tricking the view_attachment into working. As this only works with customizations that already are required to handle their own security of an attachment, this isn't a security issue. But it is causing unexpected problems because attachments from customizations can't be loaded without topic in the url to trigger the board permission to be loaded.

@Sesquipedalian, Do you feel this is a SMF bug or not? I'm thinking it is, but at the same time, I can also hook into integrate_pre_download_request and then do my own check (again) and then set $_SESSION['attachments_can_preview'][$attachId] which will allow the attachment to be viewed normally.

[sbulen]

Synchronicity... I was just looking at this for some custom code used on my forum...

Yes, boards are checked before the 'any' parameter, and if null will always return before even looking at the 'any' parameter.

FYI, my workaround was to use boardsAllowedTo, which happened to work out better for us; knowing which boards is a good thing. You actually get more info...

[jdarwood007]

Yea, I'm not touching anything from the customization yet. I have a url with the idAttach in it and using the dlattach action. the hook I have, doesn't even get executed because this check happens before the hook executes.

[sbulen]

That logic behaves ~the same in 2.0.

So that mod hasn't worked outside of a board context, ever, I think...

[jdarwood007]

in 2.0 it was using a edit to the attachments system and it worked. I will work on logic in my customization to work around this. I think about it more and the proper fix is something that 2.1 can't handle in a patch. Its best left up to a new branch.

[Sesquipedalian]

This is definitely a bug in SMF. Looks like it has been around since at least b3162a2.

We should fix it, but it will need thorough testing to make sure the fix doesn't break something else.

[Sesquipedalian]

From what I can tell, the idea behind requiring a topic URL parameter was to make sure that the user could view attachments in the board that contains the attachment's post. This is pointless in terms of security. We should drop all checks against the topic URL parameter and simply run our checks against the user's permissions in the board that the attachment's post is actually in.