Skip to content

Roles mode serves every item's title and pin link to visitors holding no admin role #1602

Description

@azuretek

Summary

With AUTH_ROLES_ENABLE=true, the dashboard filters tiles by the caller's roles correctly, but the sidenav pin list beside it is neither filtered nor gated. Every visitor, whatever roles they hold, is served the title and item id of every item in the database plus an items.pintoggle link for each one. The #add-item control that opens that list is ungated too, while #config-buttons a few lines below it is correctly gated on the admin role.

On our instance a visitor holding a single non-admin role receives 6 dashboard tiles and 16 sidenav entries.

Root cause

Two gaps on the same screen.

app/Http/Controllers/ItemController.php, dash(), roles branch. apps is role-filtered and all_apps is not:

$data['apps'] = Item::whereHas('parents', function ($query) {
    $query->where('id', 0);
})->whereIn('role', $roles)->orWhere('type', 1)->pinned()->orderBy('order', 'asc')->get();

$data['all_apps'] = Item::whereHas('parents', function ($query) {
    $query->where('id', 0);
})->orWhere('type', 1)->orderBy('order', 'asc')->get();

resources/views/layouts/app.blade.php renders all_apps for everyone, gated only on isset:

<nav class="sidenav">
    <a class="close-sidenav" href=""><i class="fas fa-times-circle"></i></a>
    @if(isset($all_apps))
    <h2>{{ __('app.dash.pinned_items') }}</h2>
    <ul id="pinlist">
        @foreach($all_apps as $app)
        <li>{{ $app->title }}<a ... href="{{ route('items.pintoggle', [$app->id]) }}">

Twelve lines further down the same template gets it right:

@if($enable_auth_admin_controls)
<div id="config-buttons">

resources/views/add.blade.php renders #add-item, which opens the pin list, with no gate at all.

Observed

Heimdall 2.8.2 (linuxserver/heimdall:2.8.2-ls362), roles supplied by caddy-security as X-Token-User-Roles. One request, one non-admin role:

$ curl -s -H 'X-Token-User-Roles: authp/media' http://127.0.0.1:8080/ > out.html

$ grep -c '<div class="item"' out.html
5
$ sed -n '/<ul id="pinlist">/,/<\/ul>/p' out.html | grep -c '<li>'
16
$ grep -oE 'href="[^"]*pintoggle[^"]*"' out.html | head -4
href="http://127.0.0.1:8080/items/pintoggle/0"
href="http://127.0.0.1:8080/items/pintoggle/6"
href="http://127.0.0.1:8080/items/pintoggle/14"
href="http://127.0.0.1:8080/items/pintoggle/4"
$ grep -c 'id="config-buttons"' out.html
0
$ grep -c 'id="add-item"' out.html
1

#config-buttons is absent, which is correct, and #add-item is present in the same response. That pair is the clearest statement of the bug: the admin check is being applied to one control and not the one beside it.

Reproduce

  1. Set AUTH_ROLES_ENABLE=true, AUTH_ROLES_HEADER, AUTH_ROLES_HTTP_HEADER, AUTH_ROLES_ADMIN and AUTH_ROLES_DELIMITER.
  2. Create two items with different values in role, neither of them the admin role.
  3. Request / with the roles header set to one of those roles.
  4. The dashboard shows only that role's tile, which is correct.
  5. The sidenav pin list contains both, plus every other item in the database, each with a pintoggle link. #add-item is present.

Impact

Disclosure of every item's title and id to every visitor is the floor.

Whether the leaked links are also actionable depends on the Heimdall user's password. UsersSeeder creates the default user with password = null, and CheckAllowed returns early for any passwordless user before it reaches any other check:

// Continue with passwordless user
if (empty($current_user->password)) {
    return $next($request);
}

ItemController applies that middleware to pinToggle like everything else, so on a default install a non-admin can follow those links and pin or unpin any item, which changes the dashboard for everyone. On an install where a Heimdall password is set the write is refused with a redirect to /login, which is what we see on ours, and the issue is disclosure only.

Why it survived

Roles mode is the only configuration in which a visitor is not an admin. Everywhere else AppServiceProvider sets enable_auth_admin_controls to true unconditionally, so the pin list is correct by construction and no amount of use in the default setup can show the difference. The dashboard query grew a role filter when the feature landed; the pin list beside it reads a different variable and did not.

Proposed fix

Gate the pin list and #add-item on $enable_auth_admin_controls, the check already used for #config-buttons. Pinning is global item state in roles mode rather than a per-visitor preference, so it belongs behind the admin check with the other controls that write global state.

Role-filtering all_apps is the other option and is not sufficient by itself. It would still leave a non-admin able to unpin their own tiles for everyone, and it takes away an admin's ability to unpin an item outside their own roles, since the pin list is the only place that can be done.

PR to follow.

Adjacent, not covered by the fix

In the same loop:

if($app->title == 'app.dashboard') continue;

never matches. Item has a title accessor that returns __('app.dashboard') when the stored value is app.dashboard, so by the time the comparison runs the value is the translated string. The home dashboard row therefore renders in the pin list as a normal entry. Cosmetic, and worth a separate look.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions