docs: map existing codebase
- stack.md (68 lines) - PHP/MySQL/Apache stack, vendored libraries - architecture.md (131 lines) - Custom MVC CMS, dual-layer (front/admin) - structure.md (170 lines) - Directory layout and conventions - conventions.md (98 lines) - PHP snake_case, SCSS $c/$f prefixes, jQuery patterns - testing.md (49 lines) - No automated tests detected - integrations.md (111 lines) - Google Maps, PHPMailer, Pixieset, Facebook - concerns.md (150 lines) - Critical security issues: hardcoded creds, MD5, unserialize - db_schema.md (260 lines) - ~32 tables with pp_ prefix, inferred from source - tech_changelog.md (9 lines) - Initial log entry Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
188
.paul/codebase/concerns.md
Normal file
188
.paul/codebase/concerns.md
Normal file
@@ -0,0 +1,188 @@
|
||||
# Codebase Concerns
|
||||
|
||||
**Analysis Date:** 2026-05-05
|
||||
|
||||
## Security Considerations
|
||||
|
||||
**Hardcoded database credentials in version control:**
|
||||
- Risk: Database password and host exposed in repository; any repo access = DB access
|
||||
- Files: `config.php` (primary), also required in `ajax.php`, `admin/ajax.php`, `admin/index.php`, `download.php`, `api/contact_map.php`, `index.php`
|
||||
- Current mitigation: None
|
||||
- Fix: Move to `.env` file, add `.env` to `.gitignore`, create `.env.example`
|
||||
|
||||
**Hardcoded Google reCAPTCHA secret key:**
|
||||
- Risk: Secret key `6Lfaovgl...` hardcoded and repeated 8 times in one file
|
||||
- Files: `plugins/special-actions-middle.php` (lines ~242, 296, 453, 531, 602, 679, 745)
|
||||
- Current mitigation: None
|
||||
- Fix: Move to config/settings, extract to single constant or `pp_settings` entry
|
||||
|
||||
**MD5 password hashing:**
|
||||
- Risk: MD5 is cryptographically broken — rainbow table attacks trivial
|
||||
- Files: `autoload/admin/factory/class.Users.php`, `autoload/admin/class.Site.php`
|
||||
- Current mitigation: None
|
||||
- Fix: Replace with `password_hash()` (bcrypt) + `password_verify()`
|
||||
|
||||
**PHP object injection via `unserialize()` on cookie data:**
|
||||
- Risk: Attacker-controlled cookie triggers PHP object injection / RCE
|
||||
- Files: `admin/ajax/pages.php` (`unserialize($_COOKIE['cookie_menus'])`, `unserialize($_COOKIE['cookie_pages'])`)
|
||||
- Also in: `admin/templates/articles/article-edit.php`, `admin/templates/layouts/layout-edit.php`, `admin/templates/pages/pages-list.php`, `admin/templates/pages/pages-browse-list.php`
|
||||
- Current mitigation: None
|
||||
- Fix: Replace with `json_decode()` for cookie state storage
|
||||
|
||||
**Path traversal in file download:**
|
||||
- Risk: Arbitrary file read — attacker can request any file on server
|
||||
- Files: `get_file.php` (no validation on `$_GET['fileUrl']` before `readfile()`)
|
||||
- Current mitigation: None
|
||||
- Fix: Whitelist allowed paths, validate against upload directory only
|
||||
|
||||
**File upload without MIME type validation:**
|
||||
- Risk: Executable files (`.php`) may be uploaded with double extension bypass
|
||||
- Files: `plugins/special-actions-middle.php` (lines ~313-316, `move_uploaded_file()` with unsanitized `$_FILES['files']['name']`)
|
||||
- Current mitigation: Extension check only (bypassable)
|
||||
- Fix: Validate MIME type via `finfo_file()`, restrict upload directory execution, add file size limits
|
||||
|
||||
**SQL injection risk via string concatenation:**
|
||||
- Risk: Direct variable injection into SQL strings (not all queries use Medoo parameterization)
|
||||
- Files:
|
||||
- `autoload/front/factory/class.Languages.php:19` — `domain` parameter in raw SQL
|
||||
- `autoload/admin/factory/class.Pages.php:353` — `lang` variable in raw SQL
|
||||
- `autoload/admin/factory/class.Articles.php:149,163,181` — multiple concatenations
|
||||
- Current mitigation: Medoo handles most queries safely; raw SQL in edge cases
|
||||
- Fix: Replace raw SQL with Medoo parameterized queries throughout
|
||||
|
||||
**User input echoed without `htmlspecialchars()`:**
|
||||
- Risk: XSS (Cross-Site Scripting) in email confirmations and form outputs
|
||||
- Files: `ajax.php:64,82-84`, `plugins/special-actions-middle.php:253-256`
|
||||
- Current mitigation: None
|
||||
- Fix: Apply `htmlspecialchars($value, ENT_QUOTES)` before output
|
||||
|
||||
## Tech Debt
|
||||
|
||||
**No environment configuration separation:**
|
||||
- Issue: Single `config.php` used for dev and production; no `.env` pattern
|
||||
- Files: `config.php` and all entry points that require it
|
||||
- Impact: Credentials hardcoded, can't safely commit config, no staging/prod separation
|
||||
- Fix: Adopt `.env` pattern with `vlucas/phpdotenv` or equivalent
|
||||
|
||||
**God object `class.S.php`:**
|
||||
- Issue: 1328-line class handling images, caching, sessions, email, DB utilities — no single responsibility
|
||||
- Files: `autoload/class.S.php`
|
||||
- Impact: Hard to test, hard to modify, used everywhere so any bug is widespread
|
||||
- Fix: Gradually extract into dedicated service classes (`ImageService`, `CacheService`, `EmailService`)
|
||||
|
||||
**Duplicate reCAPTCHA verification logic:**
|
||||
- Issue: reCAPTCHA verification code copy-pasted 8+ times instead of a shared function
|
||||
- Files: `plugins/special-actions-middle.php` (lines 230-809, repeated blocks)
|
||||
- Impact: Any bug fix requires 8 changes; any key rotation requires 8 updates
|
||||
- Fix: Extract to `verify_recaptcha($response)` function called once per form
|
||||
|
||||
**Error suppression instead of error handling:**
|
||||
- Issue: `error_reporting(0)` silences all errors; no logging to file or monitoring
|
||||
- Files: `admin/ajax.php:2`, `admin/index.php:14`
|
||||
- Impact: Silent failures, impossible to debug production issues
|
||||
- Fix: Implement proper error logging (`error_log()` or PSR-3 logger), use try/catch in DB operations
|
||||
|
||||
**Mixed concerns in templates:**
|
||||
- Issue: Business logic and data manipulation embedded in template PHP files
|
||||
- Files: `admin/templates/articles/article-edit.php` (1143 lines), `admin/templates/pages/page-edit.php`
|
||||
- Impact: Hard to maintain, duplicate logic between templates and factory classes
|
||||
- Fix: Move all logic to controls/factory, pass pre-computed variables to templates
|
||||
|
||||
**Deprecated function usage:**
|
||||
- Issue: `mime_content_type()` deprecated since PHP 5.3
|
||||
- Files: `autoload/class.S.php:37`
|
||||
- Fix: Replace with `finfo_file()`
|
||||
|
||||
## Performance Bottlenecks
|
||||
|
||||
**Large data file loaded at runtime:**
|
||||
- Problem: `wojewodztwa.php` is 6548 lines of PHP arrays — loaded entirely for any request needing province data
|
||||
- Files: `wojewodztwa.php`
|
||||
- Cause: Static data embedded in PHP instead of database or JSON
|
||||
- Fix: Move to `pp_provinces` table or JSON file with lazy loading
|
||||
|
||||
**Template placeholder replacement:**
|
||||
- Problem: Layout HTML scanned for all placeholder patterns on every page request
|
||||
- Files: `autoload/front/view/class.Site.php`
|
||||
- Cause: String replacement for `[MENU:id]`, `[ARTYKULY:id]`, etc. on each render
|
||||
- Improvement: Page caching (already implemented as opt-in, ensure enabled for high-traffic pages)
|
||||
|
||||
**WebP image generation:**
|
||||
- Problem: WebP conversion happens on-demand per request (not pre-generated)
|
||||
- Files: `autoload/class.Image.php`, `autoload/class.S.php`
|
||||
- Cause: No background job system for image processing
|
||||
- Impact: First request for each image is slow; cache/ fills over time
|
||||
- Improvement: Pre-generate WebP on upload
|
||||
|
||||
## Fragile Areas
|
||||
|
||||
**Plugin hook file `special-actions-middle.php`:**
|
||||
- Files: `plugins/special-actions-middle.php` (very large file with 8+ contact form handlers)
|
||||
- Why fragile: Monolithic — all contact forms, reCAPTCHA, file uploads in one file; no shared validation
|
||||
- Common failures: Adding a new form variant requires duplicating entire handler block
|
||||
- Safe modification: Extract shared validation/email logic before adding new variants
|
||||
|
||||
**Template override system:**
|
||||
- Files: `autoload/class.Tpl.php`, `templates/`, `templates_user/`
|
||||
- Why fragile: Silent fallback from `templates_user/` to `templates/` — easy to edit wrong file
|
||||
- Common failures: Edit `templates/` file thinking it's active, but `templates_user/` override takes precedence
|
||||
- Safe modification: Always check both directories; `templates_user/` takes precedence
|
||||
|
||||
**Admin session / cookie auto-login:**
|
||||
- Files: `admin/index.php` (lines 36-84)
|
||||
- Why fragile: IP-based session validation can lock out admin on IP change; cookie auto-login not encrypted
|
||||
- Common failures: Admin locked out after ISP IP change
|
||||
- Safe modification: Test login flow after any changes to session handling
|
||||
|
||||
## Dependencies at Risk
|
||||
|
||||
**jQuery 1.11.1 (admin panel):**
|
||||
- Risk: EOL since 2016; known XSS vulnerabilities
|
||||
- Impact: Admin panel DOM manipulation, CKEditor compatibility
|
||||
- Files: `admin/templates/site/main-layout.php`
|
||||
- Migration: Upgrade to jQuery 3.x (breaking changes in `.live()`, `.size()` etc.)
|
||||
|
||||
**CKEditor (version unknown):**
|
||||
- Risk: Older CKEditor 4.x versions have known XSS vulnerabilities
|
||||
- Files: `libraries/ckeditor/`
|
||||
- Migration: Audit version, update to latest CKEditor 4 LTS or migrate to CKEditor 5
|
||||
|
||||
**Medoo (version unknown, likely 1.x):**
|
||||
- Risk: Medoo 1.x API differs significantly from 2.x; unmaintained in 1.x branch
|
||||
- Files: `libraries/medoo/medoo.php`
|
||||
- Migration: Audit API usage before upgrading to Medoo 2.x
|
||||
|
||||
## Missing Critical Features
|
||||
|
||||
**No environment configuration:**
|
||||
- Problem: No way to run app locally without overwriting production credentials
|
||||
- Blocks: Safe local development, CI/CD setup, multi-developer workflow
|
||||
- Complexity: Low — add `.env` loading at top of `config.php`
|
||||
|
||||
**No error logging / monitoring:**
|
||||
- Problem: Production errors are silently swallowed
|
||||
- Blocks: Debugging production issues, alerting on failures
|
||||
- Complexity: Low — configure `error_log()` to file + optional email alert
|
||||
|
||||
**No CSRF protection:**
|
||||
- Problem: All forms lack CSRF token validation
|
||||
- Blocks: Prevents CSRF attacks on contact/newsletter/admin forms
|
||||
- Complexity: Medium — add token generation + validation middleware
|
||||
|
||||
**No automated tests:**
|
||||
- Problem: Zero test coverage — no regression safety net for changes
|
||||
- Blocks: Refactoring, safe dependency upgrades, CI/CD
|
||||
- Complexity: High — requires setting up PHPUnit, test DB, writing tests from scratch
|
||||
|
||||
## Test Coverage Gaps
|
||||
|
||||
**Entire codebase:**
|
||||
- What's not tested: Everything — factory methods, controls, views, AJAX handlers
|
||||
- Risk: Any change could break functionality silently
|
||||
- Priority: High for security-sensitive paths (auth, file uploads, SQL queries)
|
||||
- Difficulty: High — no test infrastructure exists; factory methods use static Medoo instance
|
||||
|
||||
---
|
||||
|
||||
*Concerns audit: 2026-05-05*
|
||||
*Update as issues are fixed or new ones discovered*
|
||||
Reference in New Issue
Block a user