Skip to content

[Security] Missing authorization checks on management outfit endpoints + SQL truncation risk #1

@sudorest

Description

@sudorest

Summary

After a thorough code review, I found two critical authorization bypasses in the management outfit system and a data corruption risk in the SQL schema that can cause silent outfit save failures — especially now that the resource uses DLC collection data.


1. Missing permission check on deleteManagementOutfit (Critical)

File: server/main.lua

The illenium-appearance:server:deleteManagementOutfit event handler only validates that the id parameter is a number, but never checks whether the calling player has permission (job membership, boss rank, or any ACE) to delete the outfit:

RegisterNetEvent('illenium-appearance:server:deleteManagementOutfit', function(id, type)
    local src = source
    if type(id) ~= 'number' then
        return DropPlayer(src, 'Illegal Action')
    end
    -- Immediately deletes — no job/rank/boss check
    MySQL.query.await('DELETE from management_outfits WHERE id = ?', { id })
end)

Impact: Any connected player who knows (or brute-forces) a management outfit ID can delete any job/gang management outfit from the database. This is trivially exploitable since outfit IDs are sequential integers.

Expected behavior: The handler should verify:

  1. The player belongs to the job/gang the outfit is associated with
  2. The player has an appropriate rank (e.g., boss/management level)

2. Insufficient permission check on saveManagementOutfit (Critical)

File: server/main.lua

The illenium-appearance:server:saveManagementOutfit event handler checks that the player's job name matches the outfit's JobName, but does not validate the player's rank or boss status:

RegisterNetEvent('illenium-appearance:server:saveManagementOutfit', function(outfitData, type)
    local src = source
    local job = type == 'job' and QBX.GetPlayer(src).PlayerData.job or QBX.GetPlayer(src).PlayerData.gang

    if job.name ~= outfitData.JobName then
        return DropPlayer(src, 'Illegal Action')
    end
    -- Any employee can create management outfits — no rank/boss check
    MySQL.insert.await(...)
end)

Impact: Any employee of a job (even the lowest rank) can create management outfits for their entire organization. Management outfits are intended to be controlled by bosses/managers only.

Expected behavior: Add a rank or boss-level permission check, for example:

if not job.isboss then
    return DropPlayer(src, 'Illegal Action')
end

Or use a configurable minimum grade threshold.


3. SQL VARCHAR columns will silently truncate DLC outfit data

File: sql/sql.sql

The player_outfits table uses fixed-width columns:

`props` varchar(1000) DEFAULT NULL,
`components` varchar(1500) DEFAULT NULL,

The resource now saves DLC collection names alongside drawable/texture data (via game/util.lua's getPedComponents/getPedProps). Each component entry can include a collectionHash string like "mp_m_freemode_01_clothe_18". With 12 components, each carrying collection metadata, the JSON payload can easily exceed 1500 characters.

Impact: Players who wear DLC clothing combinations will have their outfit data silently truncated by MySQL, resulting in corrupted JSON that cannot be parsed back — the outfit is permanently broken. This is a data-loss bug that's invisible to the user at save time but manifests as failures when loading.

Fix: Change both columns to TEXT:

ALTER TABLE player_outfits MODIFY COLUMN props TEXT DEFAULT NULL;
ALTER TABLE player_outfits MODIFY COLUMN components TEXT DEFAULT NULL;

This matches how the playerskins table already handles it (skin TEXT).


Bonus: Minor issues found during review

Issue File Severity
ChangeRoutingBucket/ResetRoutingBucket events lack server-side state validation (any client can move themselves between routing buckets at will) server/main.lua Medium
math.randomseed(os.time()) in NanoID generator — second-resolution seed makes outfit codes predictable across servers server/util.lua Low
LoadJobOutfit uses legacy SetPedComponentVariation instead of collection-aware setPedComponent, bypassing DLC stability improvements client/outfits.lua Low

Environment

  • Resource version: v5.7.0 Qbox rewrite (latest commit as of this report)
  • Framework: qbx_core

Happy to submit a PR for any/all of these if you'd like. The security fixes (#1 and #2) are straightforward and high-priority.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions