bilety.brzezovka.pl/.paul/codebase/concerns.md

# Codebase Concerns

**Analysis Date:** 2026-04-26

## Critical Security Issues

**SQL Injection — Admin Panel:**
- `autoload/controls/class.Apanel.php` (line ~34)
- `$_GET['id']` concatenated directly into raw SQL: `'SELECT * FROM order_tickets WHERE order_id =' . $clientId`
- Any authenticated admin (or CSRF attacker) can inject arbitrary SQL
- Fix: Use Medoo parameterized query: `$mdb->select('order_tickets', '*', ['order_id' => (int)$clientId])`

**Hardcoded Credentials in Source Control:**
- `config.php` — DB password, SMTP password (`biletyonline`), admin password (`Admin2022!`), Przelewy24 CRC key, fakturowo.pl API key, test mode secret
- Any repo access exposes production credentials
- Fix: Move to environment variables or a non-committed config file

**Weak Password Hashing (MD5):**
- `autoload/controls/class.Users.php` and `autoload/factory/class.Users.php`
- User passwords stored and compared as plain MD5 — trivially broken by rainbow tables
- Fix: Replace with `password_hash()` / `password_verify()` (bcrypt)

**Payment Callback Not Fully Verified:**
- `autoload/controls/class.Tickets.php::przelewy24_response()`
- CRC check exists but cURL verification call return value may not be checked before marking order paid
- Fix: Ensure verification response is confirmed successful before updating `payment_status`

## High Priority

**Missing Input Validation on Checkout Form:**
- `autoload/controls/class.Tickets.php::basketFormHandler()`
- `$_POST` fields (email, name, NIP, ZIP) trimmed but not format-validated before DB insert
- Email not validated with `filter_var($email, FILTER_VALIDATE_EMAIL)`
- NIP, ZIP not validated for Polish format
- Fix: Add validation layer before processing order

**888-Line Monolithic Controller:**
- `autoload/controls/class.Tickets.php` — 888 lines, handles 15+ distinct operations
- Mixes: ticket display, cart management, payment processing, email sending, invoice generation, QR generation, GA data layer
- Makes changes risky (large surface area, no tests)
- Fix: Extract into `TicketsCart`, `TicketsCheckout`, `TicketsPayment`, `TicketsNotification`

**No CSRF Protection on Admin Actions:**
- `autoload/controls/class.Apanel.php` — all POST actions (login, order edits, deletes)
- No CSRF tokens on any form
- Fix: Add token generation + validation middleware

**cURL Error Handling Missing:**
- `autoload/controls/class.Tickets.php` — Przelewy24 and fakturowo.pl cURL calls
- `curl_exec()` return value not consistently checked; no timeout set
- Silent failures possible: order could be marked paid without invoice, or invoice not generated
- Fix: Check return value, add `CURLOPT_TIMEOUT`, log failures

**No Session Fixation Protection on Login:**
- `autoload/controls/class.Users.php::login()` — no `session_regenerate_id(true)` on successful auth
- Session regeneration only on first visit (new session), not on privilege escalation
- Fix: Call `session_regenerate_id(true)` after successful login

## Medium Priority

**Hardcoded Domain URLs:**
- `autoload/controls/class.Tickets.php` — `https://bilety.brzezovka.pl` hardcoded in ~5 places
- Breaks if domain changes; makes staging difficult
- Fix: Store base URL in `config.php` as `$settings['base_url']`

**No Application Logging:**
- No error logging system (PSR-3 or equivalent)
- No audit trail for payments, admin actions, or errors
- Only optional `mail_debug.log` for email
- Fix: Add basic file logger for critical operations (payment events, admin changes)

**Weak Order Hash Generation:**
- `autoload/controls/class.Tickets.php` — hash based on MD5 of email + city + timestamp
- 1-second collision window for same customer; predictable construction
- Fix: Use `bin2hex(random_bytes(16))` for cryptographically random order hashes

**Global Variables Without DI:**
- All controllers: `global $settings, $mdb, $user` at top of every method
- Untestable, hidden dependencies, risk of undefined global
- Fix: Long-term — dependency injection; short-term — document as known pattern

**QR Codes in Web-Accessible Directory:**
- `orders/{hash[0]}/{hash[1]}/{hash}.png` — served directly by Apache
- Hash is predictable (see hash concern above)
- Fix: Move QR storage outside web root; serve via PHP controller with auth check

**Large Template Files:**
- `templates/tickets/main-view.php` — 401 lines
- `templates/admin-panel/order-data.php` — 373 lines
- `templates/tickets/basket-view.php` — 316 lines
- Mixed PHP logic and HTML; difficult to maintain
- Fix: Extract repeated logic to view helper classes

**Duplicate Bootstrap Code:**
- `index.php`, `ajax.php`, `api.php`, `cron.php` — each contains near-identical DB init, session, autoloader
- Fix: Extract to `bootstrap.php` included by all entry points

**No Error Handling on DB Writes:**
- `autoload/controls/class.Tickets.php` — `$mdb->insert()` / `$mdb->update()` calls without checking return value
- Silent failure if DB write fails (order not saved but user gets confirmation)
- Fix: Check return value; wrap in try/catch

## Low Priority / Documentation

**Magic Numbers in Pricing Logic:**
- `autoload/controls/class.Tickets.php` — Day ranges `0`, `2`, `7` for dynamic pricing; product ID `999999` for delivery
- Fix: Define as named constants (e.g., `const DELIVERY_PRODUCT_ID = 999999`)

**Commented Debug Code:**
- `autoload/controls/class.Tickets.php` — `// file_put_contents('sandbox_przelewy24_response.txt', ...)` left in code
- Fix: Remove

**Short PHP Tags in Templates:**
- `templates/` — `<? if ...` (deprecated, requires `short_open_tag = On`)
- Fix: Replace with `<?php` in templates over time

**MD5 Session CSRF Token:**
- `index.php` — `$_SESSION['check'] = md5(...)` used as CSRF-adjacent token
- MD5 for security tokens is considered weak
- Fix: Use `bin2hex(random_bytes(32))` for session tokens

**No README / Setup Guide:**
- No documentation on how to set up dev environment or database
- No `.env.example` or credential template
- Fix: Add `README.md` with setup steps and required config values

---

*Concerns analysis: 2026-04-26*
*Priority order: Critical → High → Medium → Low*
*Address Critical items before any new feature development*