-
Notifications
You must be signed in to change notification settings - Fork 3
Description
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:
- The player belongs to the job/gang the outfit is associated with
- 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')
endOr 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.