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