# Security Notes

## Tenant-isolation issue found & fixed during testing

While live-testing the REST API with two seeded schools, a real
cross-tenant write vulnerability was found and fixed before delivery:

**Issue**: `erp_attendance` has a unique key of `(student_id,
attendance_date)` — it does not include `school_id`. The original
`api/attendance/mark.php` and the admin panel's `attendance/mark.php`
trusted `student_id` values from the request without verifying they
belonged to the caller's own school. Because the INSERT used
`ON DUPLICATE KEY UPDATE`, a malicious or misconfigured client
authenticated as **School B** could submit School A's `student_id` and
silently overwrite School A's attendance record for that day.

**Verified with a live test**: seeded two separate schools, marked School
A's student "absent" as School A's admin, then — as School B's admin —
attempted to mark the same `student_id` "present". Before the fix, this
silently overwrote School A's row. After the fix, School B's write is
validated against `erp_students WHERE id=? AND school_id=? AND
class_id=? AND section_id=?` before it's allowed to proceed, and
non-matching records are skipped rather than written.

**Same pattern audited and fixed in**:
- `api/attendance/mark.php` (bulk records branch — RFID single-scan branch
  was already safe, it looks up the student by `school_id` first)
- `admin/school-admin/attendance/mark.php` (same fix, web panel)
- `api/results/enter.php` (exam ownership + student ownership now both
  verified before write)
- `admin/school-admin/results/entry.php` (student-in-class validated
  before write)
- `api/homework/create.php` (class/section/subject/teacher ownership
  verified before insert)
- `admin/school-admin/teachers/view.php` subject-assignment form
  (section/subject ownership verified — lower severity, this table's
  unique key already includes `teacher_id` which is inherently
  school-scoped, so this was a data-integrity fix rather than a
  cross-tenant overwrite risk)
- `admin/school-admin/exams/add.php` (class + subject-list ownership
  verified — same lower-severity data-integrity category)

**Why other endpoints were safe by construction**: tables like
`erp_results` and `erp_exam_subjects` key on `exam_id` (which is itself
globally unique and always created under one school), so as long as the
*other* half of the composite key (student_id, subject_id) is validated,
no cross-tenant collision is possible — there's no scenario where two
different schools' legitimate rows share the same `exam_id`. Read
endpoints (`GET /api/fees/status.php`, `results/get.php`, etc.) were
audited and are safe because they filter by `school_id` **and** the
resource ID together — a mismatched combination returns zero rows, not
another tenant's data.

## Hardening already in place

- **SQL injection**: 100% PDO prepared statements throughout (verified via
  a full-codebase grep for `->query()` calls with any risk of unescaped
  string interpolation — the only interpolated values found were
  `(int)`-cast session-derived or already-validated integers, e.g.
  `school_id`, `plan_id`, pagination `limit`/`offset`).
- **XSS**: every admin-panel output goes through the `e()` escaping helper;
  no raw `$_POST`/`$_GET`/DB values are echoed unescaped.
- **CSRF**: session cookies are set with `SameSite=Lax`, which blocks the
  browser from sending the session cookie on cross-site POST requests —
  this covers the standard CSRF threat model for the admin panels. Token-
  based CSRF helpers (`csrfToken()`/`verifyCsrf()` in
  `includes/functions.php`) are available if you want to layer on
  double-submit-cookie protection for extra-sensitive actions (e.g. school
  deletion already requires typing the school code as a confirmation,
  which serves a similar purpose).
- **File uploads**: validated by real MIME type (`finfo`, not just
  extension or client-supplied `Content-Type`), renamed to a UUID on save
  (never trusts the original filename), and `uploads/.htaccess` disables
  PHP execution in that folder entirely — even if a malicious file somehow
  got past MIME validation, it cannot be executed as a script.
- **Password storage**: `password_hash()`/`password_verify()` (bcrypt),
  never plaintext, never reversible encryption.
- **JWT**: HS256, hand-rolled (no Composer dependency), tested for
  signature-tamper rejection and expiry enforcement. Change
  `JWT_SECRET` before deploying — the shipped value is a placeholder.
- **Rate limiting**: login attempts are throttled (5 failures / 15 minutes)
  on both the super-admin panel and the API login endpoint.
- **Directory protection**: `.htaccess` denies direct web access to
  `config/`, `includes/`, `cron/`, `backups/`, `database/`.
- **Authorization header pass-through**: root `.htaccess` includes the
  `mod_rewrite` fix for hosts that strip the `Authorization` header on
  CGI/suPHP configurations (a common silent failure mode for JWT auth on
  shared hosting) — `includes/auth_middleware.php` also has an
  `apache_request_headers()` fallback.

## Recommended before go-live

- [ ] Change `JWT_SECRET` in `config/constants.php`
- [ ] Change the seeded super admin password
- [ ] Set `APP_DEBUG` to `false`
- [ ] Wire up SMS/SMTP delivery for the password-reset code (currently
      generated and stored, but not sent anywhere — see `INSTALLATION.md` §9)
- [ ] Configure the Firebase service account for push notification
      delivery (`cron/send-push-notifications.php`)
- [ ] Review `MAX_LOGIN_ATTEMPTS` / `LOGIN_LOCKOUT_MINUTES` in
      `config/constants.php` for your risk tolerance
- [ ] If you add new write endpoints later (API or admin panel), follow the
      same pattern: never trust a foreign-key ID from the request without
      first checking it belongs to `Auth::schoolId()` / the session's
      `school_id`