Validate availability slot datetimes (REST and admin form) #42

New Issue

2026-06-10T18:50:53Z

thatguygriff commented

2026-06-10 18:50:53 +00:00

Finding (security review — Low / robustness)

Both availability creation paths accept arbitrary start_dt / end_dt strings:

AvailabilityEndpoint::create() (src/Availability/AvailabilityEndpoint.php) — REST, sanitize_text_field only
AvailabilityController::addSlot() (src/Availability/AvailabilityController.php) — admin form (datetime-local inputs, Y-m-d\TH:i)

On the weekly path, new \DateTimeImmutable($startDt) in AvailabilityRepository::createWeeklySeries() throws on garbage → unhandled 500. On the single path the raw string lands in a DATETIME column and MySQL decides what to store. There is also no check that end_dt > start_dt.

Only authenticated instructors reach these paths, so this is robustness/data-integrity more than exploitability.

Fix

Normalise and validate in both paths: accept Y-m-d H:i[:s] and the datetime-local Y-m-d\TH:i[:s] variants, canonicalise to Y-m-d H:i:s, reject anything else and any slot where end <= start (REST → 400; admin form → no-op like the existing empty-field check). Cover with unit tests.

## Finding (security review — Low / robustness) Both availability creation paths accept arbitrary `start_dt` / `end_dt` strings: - `AvailabilityEndpoint::create()` (src/Availability/AvailabilityEndpoint.php) — REST, `sanitize_text_field` only - `AvailabilityController::addSlot()` (src/Availability/AvailabilityController.php) — admin form (`datetime-local` inputs, `Y-m-d\TH:i`) On the weekly path, `new \DateTimeImmutable($startDt)` in `AvailabilityRepository::createWeeklySeries()` throws on garbage → unhandled 500. On the single path the raw string lands in a DATETIME column and MySQL decides what to store. There is also no check that `end_dt > start_dt`. Only authenticated instructors reach these paths, so this is robustness/data-integrity more than exploitability. ## Fix Normalise and validate in both paths: accept `Y-m-d H:i[:s]` and the `datetime-local` `Y-m-d\TH:i[:s]` variants, canonicalise to `Y-m-d H:i:s`, reject anything else and any slot where end <= start (REST → 400; admin form → no-op like the existing empty-field check). Cover with unit tests.

thatguygriff added the bug security labels 2026-06-10 18:50:53 +00:00

thatguygriff referenced this issue from a commit

2026-06-10 19:36:30 +00:00

Security fixes: CSV injection, policy body output, invite hashing, slot datetimes

thatguygriff referenced this issue

2026-06-10 19:39:33 +00:00

Security fixes: CSV injection, policy body output, invite token hashing, slot datetime validation #43

thatguygriff closed this issue

2026-06-10 19:51:22 +00:00

Sign in to join this conversation.