the-outuni-project / legacy-waitlist

A fork of the TDF Waitlist developed by Nyx Viliana - former TDF HQ FC.
https://42outunis.com
MIT License
2 stars 12 forks source link

Security Vulnerability: Skill Updater stores non-relevant skills #41

Closed samuelgrant closed 1 year ago

samuelgrant commented 1 year ago

Overview

This website includes various skill tracking systems to help FCs check whether a pilot has the required skills to fly, to help produce skill plans, and give pilots a page where they can track their skill training progress.

For this system to function, a background worker which pulls character skill information from ESI and then feeds it into two database tables.

This system should only track the skills required for our skills, skill plans pages, and the x-up checker. The required skills are specified here:

However, the system currently logs ALL skills that a pilot has. This information is then made available to the FC team through the pilot profile page.

Risk Assessment

Description Risk
Database is compromised via SSH/shell. Low
Only @samuelgrant @zuntrax have shell access.
FC scrapes or visually looks up data on the profile page. Moderate
Only full FCs, instructors, and leadership have access to view other pilot's profile pages. This means only an additional 11 people can view other pilots' skills and specifically skill history*.

While we trust these users, there is no code in place to prevent API scraping OR an audit log that would allow us to check FCs are not abusing their access. **

* Skill History records skills that have been recorded in the current_skills table and have since changed again (skill trained or extracted)

** As this reason is a moderate risk, this issue should be addressed ASAP.

Who is Affected

All characters who have authenticated (using Eve SSO) and authorized the scope esi-skills.read_skills.v1 with the TOP waitlist @ https://42outunis.com

Resolution Plan

samuelgrant commented 1 year ago

Testing reveals:

samuelgrant commented 1 year ago

Deleted skills from the skill_current table, we still need to drop the skill_history table.

samuelgrant commented 1 year ago

This should drop the required skills from skill_history

START TRANSACTION;
delete from skill_history where skill_id not in (3433, 3315, 12096, 28879, 60515, 33699, 3336, 3450, 3429, 3428, 3312, 33098, 3327, 3437, 11207, 22806, 20495, 12485, 33093, 23566, 3333, 3318, 3427, 33094, 19759, 3423, 12212, 3405, 3454, 23618, 3422, 33078, 3392, 28880, 19921, 3310, 26252, 3413, 3331, 3435, 12486, 28667, 3311, 12215, 26253, 3411, 22809, 3402, 3417, 33097, 24241, 22807, 3452, 3449, 3337, 3436, 12487, 3418, 3431, 3349, 3332, 3451, 3394, 12484, 3455, 3330, 3317, 3453, 33096, 3334, 23606, 3352, 25530, 20494, 3329, 4385, 11572, 3354, 23950, 27902, 3339, 3335, 3439, 3307, 3309, 28164, 3348, 26254, 3328, 3442, 3426, 33095, 19760, 33091, 3441, 3430, 3316, 3338, 11569, 3303, 3419, 3300, 3424, 3416, 33092, 33002, 11574, 24764, 12305, 22808, 3304, 3301, 32999, 25538, 3393, 16069, 3306);
COMMIT;